Skip to content

Commit 74bc872

Browse files
authored
Fix logs with leading spaces in the Docker operator (#33692) (#43840)
Python 3.11’s multi-line error arrows don’t display correctly in Airflow’s DockerOperator logs due to leading spaces being removed, making error messages hard to read. Before fix: return self.main(*args, **kwargs) ^^^^^^^^^^^^^^^^ After fix: return self.main(*args, **kwargs) ^^^^^^^^^^^^^^^^ Fixes: #33692
1 parent 340a70b commit 74bc872

File tree

2 files changed

+45
-8
lines changed

2 files changed

+45
-8
lines changed

providers/src/airflow/providers/docker/operators/docker.py

+14-7
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@
4747
from airflow.utils.types import NOTSET, ArgNotSet
4848

4949
if TYPE_CHECKING:
50+
from logging import Logger
51+
5052
from docker import APIClient
5153
from docker.types import DeviceRequest
5254

@@ -62,6 +64,16 @@ def stringify(line: str | bytes):
6264
return line
6365

6466

67+
def fetch_logs(log_stream, log: Logger):
68+
log_lines = []
69+
for log_chunk in log_stream:
70+
log_chunk = stringify(log_chunk).rstrip()
71+
log_lines.append(log_chunk)
72+
for log_chunk_line in log_chunk.split("\n"):
73+
log.info("%s", log_chunk_line)
74+
return log_lines
75+
76+
6577
class DockerOperator(BaseOperator):
6678
"""
6779
Execute a command inside a docker container.
@@ -426,16 +438,11 @@ def _run_image_with_mounts(self, target_mounts, add_tmp_variable: bool) -> list[
426438
tty=self.tty,
427439
hostname=self.hostname,
428440
)
429-
logstream = self.cli.attach(container=self.container["Id"], stdout=True, stderr=True, stream=True)
441+
log_stream = self.cli.attach(container=self.container["Id"], stdout=True, stderr=True, stream=True)
430442
try:
431443
self.cli.start(self.container["Id"])
432444

433-
log_lines = []
434-
for log_chunk in logstream:
435-
log_chunk = stringify(log_chunk).strip()
436-
log_lines.append(log_chunk)
437-
for log_chunk_line in log_chunk.split("\n"):
438-
self.log.info("%s", log_chunk_line)
445+
log_lines = fetch_logs(log_stream, self.log)
439446

440447
result = self.cli.wait(self.container["Id"])
441448
if result["StatusCode"] in self.skip_on_exit_code:

providers/tests/docker/operators/test_docker.py

+31-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828

2929
from airflow.exceptions import AirflowException, AirflowProviderDeprecationWarning, AirflowSkipException
3030
from airflow.providers.docker.exceptions import DockerContainerFailedException
31-
from airflow.providers.docker.operators.docker import DockerOperator
31+
from airflow.providers.docker.operators.docker import DockerOperator, fetch_logs
3232
from airflow.utils.task_instance_session import set_current_task_instance_session
3333

3434
TEST_CONN_ID = "docker_test_connection"
@@ -865,3 +865,33 @@ def test_partial_deprecated_skip_exit_code_ambiguous(
865865
pytest.raises(ValueError, match="Conflicting `skip_on_exit_code` provided"),
866866
):
867867
ti.render_templates()
868+
869+
@pytest.mark.parametrize(
870+
"log_lines, expected_lines",
871+
[
872+
pytest.param(
873+
[
874+
"return self.main(*args, **kwargs)",
875+
" ^^^^^^^^^^^^^^^^",
876+
],
877+
[
878+
"return self.main(*args, **kwargs)",
879+
" ^^^^^^^^^^^^^^^^",
880+
],
881+
id="should-not-remove-leading-spaces",
882+
),
883+
pytest.param(
884+
[
885+
" ^^^^^^^^^^^^^^^^ ",
886+
],
887+
[
888+
" ^^^^^^^^^^^^^^^^",
889+
],
890+
id="should-remove-trailing-spaces",
891+
),
892+
],
893+
)
894+
@mock.patch("logging.Logger")
895+
def test_fetch_logs(self, logger_mock, log_lines, expected_lines):
896+
fetch_logs(log_lines, logger_mock)
897+
assert logger_mock.info.call_args_list == [call("%s", line) for line in expected_lines]

0 commit comments

Comments
 (0)