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

BaseExecutor allows executors to consumer task_instance parameters #44016

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

AutomationDev85
Copy link
Contributor

@AutomationDev85 AutomationDev85 commented Nov 14, 2024

Description

During implementation of the slot handling in the Edge provider package (see #43737) a discussion about how to enable executors to use additional information was started. This PR tackles the idea on extend the execute_async function with task_instance to allow executor consuming all the task_instance parameters. With this it is pretty simple to change a the interface and the executer can decide with task_instance parameters he wants to use to implement task specific behaviors.

This PR shows a possible solution how to tackle option 2 of discussion in this PR #43737. Will also create a devlist discussion about this.

Details about changes

  • Add execute function into BaseExecutor which has task_instance as parameter.
  • New execute function calls old execute_async function to keep not updated Executors running.
  • Add deprecation warning for Executor which use the old execute_async function.

@jscheffl
Copy link
Contributor

Even though it is green, converting to DRAFT as long as devlist discussion is ongoing.

@jscheffl jscheffl marked this pull request as draft November 14, 2024 19:40
Copy link
Contributor

@jscheffl jscheffl left a comment

Choose a reason for hiding this comment

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

Thanks for the proposal. I like it.

I assume the implementation is made serving the devlist discussion, before merge the following things would need to be added:

  • Documentation naming the deprecation and describing the new signature
  • Probably also some naming in the "Authoring and Scheduling" documentation or Pools documentation so that users know about the feature
  • Newsfragment highlighting the change
  • Pytests for the new API
  • Adjustment of the "Core Executors" shipped with Airflow, Sequential and Local Executor - ideally LocalExecutor could serve as a kind of reference implementation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:Executors-core LocalExecutor & SequentialExecutor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants