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

Refactor task run engine #13793

Merged
merged 3 commits into from
Jun 5, 2024
Merged

Refactor task run engine #13793

merged 3 commits into from
Jun 5, 2024

Conversation

jlowin
Copy link
Member

@jlowin jlowin commented Jun 5, 2024

This refactors the engine to create utility methods that allow very lightweight "caller" functions (e.g. run_task_sync and run_task_async) which invoke an engine to govern a given task. A major motivation of this refactor is to make it easy to extend the engine to other types of tasks (generators, contexts, and more) without needing to rewrite all of the runtime logic.

In the current implementation, you have to know exactly which engine methods to invoke and when in order to run a task - totalling 50 LOC for each of run_task_sync and run_task_async. While this refactor moves code rather than removes code, it does result in much thinner callers that are easier to extend to novel task types or situations.

@jlowin jlowin requested a review from a team as a code owner June 5, 2024 03:46
Copy link
Member

@desertaxle desertaxle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jlowin jlowin merged commit fb9f103 into main Jun 5, 2024
27 checks passed
@jlowin jlowin deleted the refactor branch June 5, 2024 16:30
@zzstoatzz zzstoatzz mentioned this pull request Jun 5, 2024
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants