You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "insomnes (via GitHub)" <gi...@apache.org> on 2025/01/09 21:39:55 UTC
[I] Allow `FileTaskHandler` get running tasks logs not only from default executor [airflow]
insomnes opened a new issue, #45529:
URL: https://github.com/apache/airflow/issues/45529
### Description
`FileTaskHandler` should have the ability to get logs not only from the default executor, but from one configured on the task instance
### Use case/motivation
With the introduction of multiple executors flow of the `FileTaskHandler` log reading of the running task was broken if the task was run by a non-default executor.
For example, usage of "LocalExecutor,KubernetesExecutor" leads to an inability of online logs reading in web interface.
I was able to overcome it with a custom executor workaround, but I belive that this would be a reasonable feature to expect with multiple executors.
### Related issues
_No response_
### Are you willing to submit a PR?
- [X] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
--
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-unsubscribe@airflow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Allow `FileTaskHandler` get running tasks logs not only from default executor [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W closed issue #45529: Allow `FileTaskHandler` get running tasks logs not only from default executor
URL: https://github.com/apache/airflow/issues/45529
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Allow `FileTaskHandler` get running tasks logs not only from default executor [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on issue #45529:
URL: https://github.com/apache/airflow/issues/45529#issuecomment-2582868483
I've been working on refactoring the log reading process recently, also focusing on `FileTaskHandler`.
I believe this can be achieved by reading directly from `ti.executor` instead of using `ExecutorLoader` to determine the correct executor method.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Allow `FileTaskHandler` get running tasks logs not only from default executor [airflow]
Posted by "o-nikolas (via GitHub)" <gi...@apache.org>.
o-nikolas commented on issue #45529:
URL: https://github.com/apache/airflow/issues/45529#issuecomment-2588254868
Hey @jason810496 thanks for the bug report! This does indeed look like it needs fixing. And you're correct that we should be looking at the current ti's executor, if it has one, otherwise the default.
You mentioned you had a workaround for this, is that something you can share or would like to contribute? Otherwise I can get started on a fix for this shortly.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Allow `FileTaskHandler` get running tasks logs not only from default executor [airflow]
Posted by "nradz (via GitHub)" <gi...@apache.org>.
nradz commented on issue #45529:
URL: https://github.com/apache/airflow/issues/45529#issuecomment-2590097952
Hello! I has also experimented this issue and I has fixed it with the following code, if it could help in the PR. I try to maintain the "cached_property", assuming that load the executors is costly. Also, I don't know if _init_executors_ is the most "elegant" way to retrieve the executors, but it is the only function that I have found to do it.
```python
def _executor_get_task_log(self, ti: TaskInstance, try_number: int) -> tuple[list[str], list[str]]:
"""
Get logs from executor.
:param ti: task instance object
:param try_number: task instance try_number to read logs from
:return: a tuple of messages and logs
"""
executor_name = ti.executor
if executor_name is None:
executor = list(self._available_executors.values())[0]
elif executor_name in self._available_executors:
executor = self._available_executors[executor_name]
else:
raise AirflowException(f"Executor {executor_name} not found for task {ti}")
return executor.get_task_log(ti, try_number)
@cached_property
def _available_executors(self) -> Dict[str, BaseExecutor]:
"""This cached property avoids loading executors repeatedly."""
return {
ex.__class__.__name__: ex
for ex in ExecutorLoader.init_executors()
}
```
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Allow `FileTaskHandler` get running tasks logs not only from default executor [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on issue #45529:
URL: https://github.com/apache/airflow/issues/45529#issuecomment-2591588678
> Also, I don't know if init_executors is the most "elegant" way to retrieve the executors, but it is the only function that I have found to do it.
Thanks @nradz, I think your solution is more cleaner and also avoid loading executor repeatedly, just adapt that for fixing this issue !
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Allow `FileTaskHandler` get running tasks logs not only from default executor [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on issue #45529:
URL: https://github.com/apache/airflow/issues/45529#issuecomment-2588561291
> You mentioned you had a workaround for this, is that something you can share or would like to contribute? Otherwise I can get started on a fix for this shortly.
Hi @o-nikolas, I can fix this issue!
By the way, the refactoring I mentioned previously is not related to this issue. It focuses on solving OOM when reading large task logs (#45129).
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Allow `FileTaskHandler` get running tasks logs not only from default executor [airflow]
Posted by "insomnes (via GitHub)" <gi...@apache.org>.
insomnes commented on issue #45529:
URL: https://github.com/apache/airflow/issues/45529#issuecomment-2581303899
I am talking about this [line](https://github.com/apache/airflow/blob/main/airflow/utils/log/file_task_handler.py#L362):
```python
if ti.state == TaskInstanceState.RUNNING:
response = self._executor_get_task_log(ti, try_number)
...
@cached_property
def _executor_get_task_log(self) -> Callable[[TaskInstance, int], tuple[list[str], list[str]]]:
"""This cached property avoids loading executor repeatedly."""
executor = ExecutorLoader.get_default_executor()
return executor.get_task_log
```
I'm not very familiar with `ExecutorLoader` and maybe the real culprit is `@cached_property` usage, but this needs some rework for multiple executors support
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Allow `FileTaskHandler` get running tasks logs not only from default executor [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on issue #45529:
URL: https://github.com/apache/airflow/issues/45529#issuecomment-2585594282
cc @o-nikolas 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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Allow `FileTaskHandler` get running tasks logs not only from default executor [airflow]
Posted by "o-nikolas (via GitHub)" <gi...@apache.org>.
o-nikolas commented on issue #45529:
URL: https://github.com/apache/airflow/issues/45529#issuecomment-2591091467
> Hi @o-nikolas, I can fix this issue!
> By the way, the refactoring I mentioned previously is not related to this issue. It focuses on solving OOM when reading large task logs (https://github.com/apache/airflow/pull/45129).
Right, perhaps we can divide and conquer then if you're working on the OOM issue then either myself or @nradz can solve this one.
> Hello! I has also experimented this issue and I has fixed it with the following code, if it could help in the PR.
Thanks for sharing this snippet! I'll work on a PR and perhaps use some of this snippet!
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Allow `FileTaskHandler` get running tasks logs not only from default executor [airflow]
Posted by "insomnes (via GitHub)" <gi...@apache.org>.
insomnes commented on issue #45529:
URL: https://github.com/apache/airflow/issues/45529#issuecomment-2613830130
Hey @jason810496 Thanks a lot for implementing this. It would be nice to drop the pile of workaround code I wrote to address this problem.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org