dstandish commented on code in PR #31101:
URL: https://github.com/apache/airflow/pull/31101#discussion_r1186452492
##########
airflow/utils/log/file_task_handler.py:
##########
@@ -317,18 +316,24 @@ def _read(
executor_messages, executor_logs = response
if executor_messages:
messages_list.extend(messages_list)
- if ti.state in (TaskInstanceState.RUNNING, TaskInstanceState.DEFERRED)
and not executor_messages:
- served_messages, served_logs = self._read_from_logs_server(ti,
worker_log_rel_path)
- messages_list.extend(served_messages)
if not (remote_logs and ti.state not in State.unfinished):
Review Comment:
I agree with you that we can improve readability of these conditions...
_however_, with recent discussions fresh in mind, after taking a few steps in
that direction (trying to refactor) i realized.... probably best to defer such
a refactor (renaming and consolidating logic for the conditionals) for a minor
release not the patch release! 😛
the only conditional added here is
```
ti.state not in State.unfinished and not (local_logs or remote_logs)
```
and this one is pretty plain
it's added as an `elif` block under the moved `if`
WDYT
--
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]
