BasPH commented on a change in pull request #18309:
URL: https://github.com/apache/airflow/pull/18309#discussion_r710834857
##########
File path: airflow/utils/log/log_reader.py
##########
@@ -54,6 +57,8 @@ def read_log_chunks(
contain information about the task log which can enable you read logs
to the
end.
"""
+ if ti.operator == DummyOpeartor.__class__.__name__:
Review comment:
Would prefer to avoid magic methods (+ typo in `DummyOperator`). Plus it
should also account for the usage of ExternalTaskMarker.
```suggestion
if isinstance(ti.operator, DummyOperator):
```
##########
File path: airflow/utils/log/log_reader.py
##########
@@ -21,9 +21,12 @@
from airflow.compat.functools import cached_property
from airflow.configuration import conf
from airflow.models import TaskInstance
+from airflow.operators.dummy import DummyOperator
from airflow.utils.helpers import render_log_filename
from airflow.utils.log.logging_mixin import ExternalLoggingMixin
+DUMMY_TASK_LOG_MESSAGE = f"tasks of type {DummyOperator.__class__.__name__} do
not have task logs \n"
Review comment:
```suggestion
DUMMY_TASK_LOG_MESSAGE = f"Tasks of type {DummyOperator.__class__.__name__}
do not have task logs.\n"
```
##########
File path: airflow/utils/log/log_reader.py
##########
@@ -70,6 +75,8 @@ def read_log_stream(self, ti: TaskInstance, try_number:
Optional[int], metadata:
:type metadata: dict
:rtype: Iterator[str]
"""
+ if ti.operator == DummyOpeartor.__class__.__name__:
Review comment:
Same here
```suggestion
if isinstance(ti.operator, DummyOperator):
```
##########
File path: tests/utils/log/test_log_reader.py
##########
@@ -238,3 +248,23 @@ def test_supports_external_link(self):
mock_prop.return_value = True
assert task_log_reader.supports_external_link
+
+ def test_test_read_log_chunks_dummy_operator(self):
+ task_log_reader = TaskLogReader()
+ logs, metadatas = task_log_reader.read_log_chunks(ti=self.dummy_ti,
try_number=1, metadata={})
+
+ assert [
+ (
+ '',
+ DUMMY_TASK_LOG_MESSAGE,
+ )
+ ] == logs[0]
Review comment:
```suggestion
assert [('', DUMMY_TASK_LOG_MESSAGE)] == logs[0]
```
##########
File path: tests/utils/log/test_log_reader.py
##########
@@ -238,3 +248,23 @@ def test_supports_external_link(self):
mock_prop.return_value = True
assert task_log_reader.supports_external_link
+
+ def test_test_read_log_chunks_dummy_operator(self):
+ task_log_reader = TaskLogReader()
+ logs, metadatas = task_log_reader.read_log_chunks(ti=self.dummy_ti,
try_number=1, metadata={})
+
+ assert [
+ (
+ '',
+ DUMMY_TASK_LOG_MESSAGE,
+ )
+ ] == logs[0]
+ assert {"end_of_log": True} == metadatas
+
+ def test_test_test_read_log_stream_should_read_one_try(self):
+ task_log_reader = TaskLogReader()
+ stream = task_log_reader.read_log_stream(ti=self.dummy_ti,
try_number=1, metadata={})
+
+ assert [
+ DUMMY_TASK_LOG_MESSAGE,
+ ] == list(stream)
Review comment:
```suggestion
assert [DUMMY_TASK_LOG_MESSAGE] == list(stream)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
