Skip to content

Move ExecutorCallback execution into a supervised process - #62645

Merged
ferruzzi merged 27 commits into
apache:mainfrom
aws-mwaa:ferruzzi/executor-callbacks/supervisor
Apr 14, 2026
Merged

Move ExecutorCallback execution into a supervised process#62645
ferruzzi merged 27 commits into
apache:mainfrom
aws-mwaa:ferruzzi/executor-callbacks/supervisor

Conversation

@ferruzzi

Copy link
Copy Markdown
Contributor

Move ExecutorCallback execution into a supervised process and make the entire flow more generic to account for future workload types

  • Generalized base workload class which both tasks and callbacks inherit (Added key, display_name, success_state, and failure_state fields)
  • Created a callback supervisor
  • Simplified/unified the shared workflow between tasks and callbacks
  • Aligned logging messages

Changes applied to both the LocalExecutor and the Celery Executor (the only executors that currently support synchronous callbacks in the worker)

Promised follow-up to #61153
promised tags for @jason810496@ashb@amoghrajesh


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@ferruzzi

Copy link
Copy Markdown
ContributorAuthor

Failures don't feel related and don't give any obvious error message, I'll try rebasing tomorrow and see if that clears them

@jscheffl

Copy link
Copy Markdown
Contributor

in-deed, these three tests are broken on main. Hope they are fixed soon, else check #ci-cd channel in Slack.

@jscheffljscheffl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Okay for Edge (just a doc change), would prefer the core SDK experts making the review.

@ferruzzi
ferruzziforce-pushed the ferruzzi/executor-callbacks/supervisor branch 2 times, most recently from c449451 to 343ee26CompareMarch 2, 2026 19:56
@o-nikolas
o-nikolas requested a review from CopilotMarch 2, 2026 20:36

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR moves synchronous ExecutorCallback execution into a supervised subprocess (similar to task execution), and generalizes executor workload handling so tasks and callbacks share a unified flow across LocalExecutor and CeleryExecutor.

Changes:

  • Introduces a callback supervisor (supervise_callback) and migrates callback execution to run in a supervised subprocess.
  • Adds generic workload metadata (key, display_name, success_state, failure_state) to unify executor handling for tasks and callbacks.
  • Aligns “workload finished” logging and updates executor/provider integrations and tests accordingly.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file
FileDescription
task-sdk/tests/task_sdk/execution_time/test_supervisor.pyUpdates expected supervisor log fields to the new “Workload finished” format.
task-sdk/tests/task_sdk/execution_time/test_callback_supervisor.pyAdds unit tests for callback importing/execution behavior.
task-sdk/src/airflow/sdk/execution_time/supervisor.pyRenames “Task finished” log event to “Workload finished” and adds workload identifiers.
task-sdk/src/airflow/sdk/execution_time/callback_supervisor.pyAdds supervised callback execution implementation and subprocess wrapper.
providers/edge3/src/airflow/providers/edge3/cli/worker.pyUpdates a reference comment to the renamed LocalExecutor helper.
providers/celery/src/airflow/providers/celery/executors/celery_executor_utils.pySwitches callback workload execution to supervise_callback.
airflow-core/tests/unit/executors/test_local_executor.pyUpdates tests for renamed LocalExecutor helper and adds callback supervisor invocation tests.
airflow-core/tests/unit/executors/test_base_executor.pyMigrates callback execution tests to use the new execute_callback helper.
airflow-core/src/airflow/executors/workloads/task.pyAdds generic workload properties to ExecuteTask.
airflow-core/src/airflow/executors/workloads/callback.pyAdds generic workload properties to ExecuteCallback and removes the old inline callback executor.
airflow-core/src/airflow/executors/workloads/base.pyIntroduces abstract workload interface (key, display_name, success/failure states).
airflow-core/src/airflow/executors/workloads/init.pyAdds ExecutorWorkload union for executor-supported workload types.
airflow-core/src/airflow/executors/local_executor.pyUnifies task/callback execution into _execute_workload and runs callbacks via supervise_callback.
airflow-core/src/airflow/executors/base_executor.pyUpdates typing to use ExecutorWorkload for workload queuing/scheduling.

Comment threadairflow-core/tests/unit/executors/test_local_executor.py Outdated
Comment threadtask-sdk/src/airflow/sdk/execution_time/callback_supervisor.py Outdated
Comment threadtask-sdk/src/airflow/sdk/execution_time/callback_supervisor.py Outdated

@o-nikolaso-nikolas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Left a few comments, on the whole it looks reasonable to me. But @ashb or @amoghrajesh should definitely look at the supervisor portion.

Comment threadairflow-core/src/airflow/executors/workloads/callback.py
Comment threadairflow-core/src/airflow/executors/base_executor.py
Comment threadtask-sdk/src/airflow/sdk/execution_time/callback_supervisor.py Outdated
@ferruzzi
ferruzziforce-pushed the ferruzzi/executor-callbacks/supervisor branch 2 times, most recently from 94e0828 to da22f0dCompareMarch 3, 2026 06:38

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.

@ashbashb added the full tests needed We need to run full set of tests for this PR to merge label Apr 14, 2026
@ashb

ashb commented Apr 14, 2026

Copy link
Copy Markdown
Member

Re-triggering with full tests just incase they weren't all running

@ashbashb closed this Apr 14, 2026
@ashbashb reopened this Apr 14, 2026

@ashbashb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM now, but please investigate the Kube test

@ferruzzi
ferruzzi merged commit ec6700d into apache:mainApr 14, 2026
407 of 408 checks passed
@ferruzzi
ferruzzi deleted the ferruzzi/executor-callbacks/supervisor branch April 14, 2026 16:37
uranusjr added a commit to astronomer/airflow that referenced this pull request May 27, 2026
uranusjr added a commit to astronomer/airflow that referenced this pull request May 27, 2026
uranusjr added a commit to astronomer/airflow that referenced this pull request May 27, 2026
uranusjr added a commit to astronomer/airflow that referenced this pull request May 27, 2026
uranusjr added a commit to astronomer/airflow that referenced this pull request May 27, 2026
uranusjr added a commit to astronomer/airflow that referenced this pull request May 27, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Executors-coreLocalExecutor & SequentialExecutorarea:providersarea:task-sdkfull tests neededWe need to run full set of tests for this PR to mergeprovider:celeryprovider:edgeEdge Executor / Worker (AIP-69) / edge3

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants

@ferruzzi@jscheffl@ashb@kaxil@shivaam@amoghrajesh@seanghaeli@o-nikolas@jason810496