uranusjr commented on code in PR #27758:
URL: https://github.com/apache/airflow/pull/27758#discussion_r1060261302
##########
airflow/utils/log/log_reader.py:
##########
@@ -34,7 +34,7 @@ class TaskLogReader:
"""Task log reader"""
def read_log_chunks(
- self, ti: TaskInstance, try_number: int | None, metadata
+ self, ti: TaskInstance, try_number: int | None, metadata, log_type=None
Review Comment:
We should make the new argument keyword-only
##########
airflow/models/trigger.py:
##########
@@ -87,7 +89,15 @@ def bulk_fetch(cls, ids: Iterable[int], session=None) ->
dict[int, Trigger]:
Fetches all of the Triggers by ID and returns a dict mapping
ID -> Trigger instance
"""
- return {obj.id: obj for obj in
session.query(cls).filter(cls.id.in_(ids)).all()}
+ return {
+ obj.id: obj
+ for obj in session.query(cls)
+ .filter(cls.id.in_(ids))
+ .options(joinedload("task_instance"))
+ .options(joinedload("task_instance.trigger"))
+ .options(joinedload("task_instance.trigger.triggerer_job"))
Review Comment:
Do we only need these for trigger log, and is it possible to not do these
for non-trigger log? (And does it make much difference in performance? I’m not
even sure about that.)
##########
airflow/utils/log/log_reader.py:
##########
@@ -55,11 +55,14 @@ def read_log_chunks(
contain information about the task log which can enable you read logs
to the
end.
"""
- logs, metadatas = self.log_handler.read(ti, try_number,
metadata=metadata)
+ kwargs = dict(log_type=log_type) if self.supports_triggerer else {}
Review Comment:
```suggestion
kwargs = {"log_type": log_type} if self.supports_triggerer else {}
```
Same for other occurrences.
##########
airflow/providers/elasticsearch/log/es_task_handler.py:
##########
@@ -187,8 +188,18 @@ def _group_logs_by_host(self, logs):
def _read_grouped_logs(self):
return True
+ @cached_property
+ def supports_triggerer(self):
+ """Not implemented yet."""
+ return False
Review Comment:
Would it be a good idea to implement this as a class variable instead? A
property returning a constant seems awkward.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
