Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow FileTaskHandler get running tasks logs not only from default executor #45529

Open
2 tasks done
insomnes opened this issue Jan 9, 2025 · 2 comments
Open
2 tasks done
Labels
area:logging area:UI Related to UI/UX. For Frontend Developers. kind:feature Feature Requests needs-triage label for new issues that we didn't triage yet

Comments

@insomnes
Copy link

insomnes commented Jan 9, 2025

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?

  • Yes I am willing to submit a PR!

Code of Conduct

@insomnes insomnes added kind:feature Feature Requests needs-triage label for new issues that we didn't triage yet labels Jan 9, 2025
@dosubot dosubot bot added area:logging area:UI Related to UI/UX. For Frontend Developers. labels Jan 9, 2025
@insomnes
Copy link
Author

insomnes commented Jan 9, 2025

I am talking about this line:

        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

@jason810496
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:logging area:UI Related to UI/UX. For Frontend Developers. kind:feature Feature Requests needs-triage label for new issues that we didn't triage yet
Projects
None yet
Development

No branches or pull requests

2 participants