Skip to content

Move CeleryExecutor to the celery provider #32526

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.

Sign up for GitHub

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

Jump to bottom
Merged
potiuk merged 1 commit into from
Jul 12, 2023
Merged

Move CeleryExecutor to the celery provider #32526

potiuk merged 1 commit into from
Jul 12, 2023

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Jul 11, 2023

Originally Celery Executor and Celery Kubernetes executor have been part of the Airflow Core. However, with AIP-51 where some of the executor internals have been decoupled from the core, we can now move the Celery (and as next step Kubernetes) executors to the providers.

This will allow to release fixes to the decoupled executors independently from releasing of the Airflow core version.

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborg boring-cyborg bot added area:CLI area:core-operators area:dev-tools area:helm-chart Airflow Helm Chart provider:cncf-kubernetes Kubernetes (k8s) provider related issues area:providers area:Scheduler including HA (high availability) scheduler area:webserver Webserver related Issues kind:documentation provider:celery labels Jul 11, 2023
@potiuk potiuk force-pushed the move-celery-executors-to-celery-provider branch from b843020 to f079d77 Compare July 11, 2023 16:47
@potiuk potiuk requested a review from eladkal July 11, 2023 16:47
@potiuk potiuk mentioned this pull request Jul 11, 2023
Merged
@potiuk
Copy link
Member Author

potiuk commented Jul 11, 2023

FYI. I arbitrary chose CeleryKubernetes to land in celery provider. It could be either. it does not REALLY matter, but I figured that we should follow the pattern in the name Celery(with Kubernetes)Executor - thus making it really a "Celery" executor with Added Kubernetes option. This also follows the pattern that CeleryKubernetesExecutor runs everything on "celery" by default, and only delegates to Kubernetes when "kubernetes" queue is used, so it seems that "Celery" is the driving force.

@potiuk
Copy link
Member Author

potiuk commented Jul 11, 2023

Eventually (when we move KubernetesExecutor to cncf.kubernetes, celery provider will (automatically) get [cncf.kubernetes] optional extra which will make >=VERSION of the cncf.kubernetes that will have the kubernetes executor included.

@potiuk potiuk force-pushed the move-celery-executors-to-celery-provider branch from f079d77 to 74d01a0 Compare July 11, 2023 17:24
@potiuk
Copy link
Member Author

potiuk commented Jul 11, 2023 •
edited
Loading

BTW. I also plan to implement Provider's manager extensions and airflow providers executors + documentation in "providers" docs once this one is merged as well.

eladkal
eladkal approved these changes Jul 11, 2023
View reviewed changes
Copy link
Contributor

@eladkal eladkal left a comment

Choose a reason for hiding this comment

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

Cool!

To clarify this is the implimitation of the vote accepted in:
https://lists.apache.org/thread/4mmzwpf9zfkkx03l1rswxfmq75hzhpk4

@potiuk potiuk force-pushed the move-celery-executors-to-celery-provider branch 3 times, most recently from 2ced796 to eab07e4 Compare July 11, 2023 21:38
@potiuk potiuk force-pushed the move-celery-executors-to-celery-provider branch 2 times, most recently from be2c31d to 000330c Compare July 12, 2023 05:56
@potiuk
Copy link
Member Author

potiuk commented Jul 12, 2023

OK. I also think I properly now raise deprecation warning and inform the users who have the old celery_app_name set, to change their application name to follow the new package name (and use the new name).

I also added app in the old "airflow.executors.celery_executor" module - following similar PEP 562 code by @o-nikolas for direct access to the "app" package.

    "celery_executor": {
        "app": "airflow.providers.celery.executors.celery_executor_utils.app",
        "CeleryExecutor": "airflow.providers.celery.executors.celery_executor.CeleryExecutor",
    },

@potiuk @jedcunningham @o-nikolas
Move CeleryExecutor to the celery provider
d50970a 
Originally Celery Executor and Celery Kubernetes executor
have been part of the Airflow Core. However, with AIP-51 where
some of the executor internals have been decoupled from the
core, we can now move the Celery (and as next step Kubernetes)
executors to the providers.

This will allow to release fixes to the decoupled executors
independently from releasing of the Airflow core version.

Co-authored-by: Jed Cunningham <66968678+jedcunningham@users.noreply.github.com>
Co-authored-by: Niko Oliveira <onikolas@amazon.com>
@potiuk potiuk force-pushed the move-celery-executors-to-celery-provider branch from 000330c to d50970a Compare July 12, 2023 06:32
@potiuk
Copy link
Member Author

potiuk commented Jul 12, 2023

Merging -> static check already fixed in main.

@potiuk potiuk merged commit 40d54ea into apache:main Jul 12, 2023
@potiuk potiuk deleted the move-celery-executors-to-celery-provider branch July 12, 2023 07:22
@pankajkoti
Copy link
Member

pankajkoti commented Jul 13, 2023 •
edited
Loading

Just curious, which wave of providers will this be released in? Or we will have some ad-hoc release for this?

We have a nightly automation which takes in the latest main branch Airflow code, but since this provider is not released we're encountering some failures here.

@eladkal
Copy link
Contributor

eladkal commented Jul 13, 2023

We have a nightly automation which takes in the latest main branch Airflow code, but since this provider is not released we're encountering some failures here.

Release manager decides which providers to release.
While providers are released from main (all or nothing, no cherry picking of commits) release manager can still decide to skip specific providers.

@pankajkoti
Copy link
Member

pankajkoti commented Jul 13, 2023

Release manager decides which providers to release. While providers are released from main (all or nothing, no cherry picking of commits) release manager can still decide to skip specific providers.

okay understood, thank you!

@potiuk potiuk mentioned this pull request Jul 15, 2023
Merged
potiuk added a commit to potiuk/airflow that referenced this pull request Jul 16, 2023
@potiuk
Move default_celery.py to inside the provider
1520b6a 
This has been missed in apache#32526 and is extracted out from apache#32604
in an attempt to make it smaller and separately reviewable.

This one adds also deprecation warning to handle the
configuration value that people might already have in
the [celery] iccelery_config_options"
potiuk added a commit that referenced this pull request Jul 16, 2023
@potiuk
Move default_celery.py to inside the provider (#32628)
ea0deaa 
This has been missed in #32526 and is extracted out from #32604
in an attempt to make it smaller and separately reviewable.

This one adds also deprecation warning to handle the
configuration value that people might already have in
the [celery] iccelery_config_options"
ferruzzi added a commit to aws-mwaa/upstream-to-airflow that referenced this pull request Jul 18, 2023
@ferruzzi
Revert "Move CeleryExecutor to the celery provider (apache#32526)"
d74c879 
This reverts commit 40d54ea
@mikaeld mikaeld mentioned this pull request Jul 21, 2023
Merged
@eladkal eladkal mentioned this pull request Jul 29, 2023
Closed
69 tasks
@ephraimbuddy ephraimbuddy added type:misc/internal Changelog: Misc changes that should appear in change log type:new-feature Changelog: New Features and removed type:misc/internal Changelog: Misc changes that should appear in change log type:new-feature Changelog: New Features labels Aug 2, 2023
@jedcunningham jedcunningham mentioned this pull request Sep 27, 2023
Closed
24 tasks
@rawwar rawwar mentioned this pull request Oct 19, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@o-nikolas o-nikolas o-nikolas left review comments

@pierrejeambrun pierrejeambrun pierrejeambrun approved these changes

@eladkal eladkal eladkal approved these changes

@jedcunningham jedcunningham jedcunningham approved these changes

@vincbeck vincbeck vincbeck approved these changes

@ashb ashb Awaiting requested review from ashb ashb is a code owner

@dstandish dstandish Awaiting requested review from dstandish

@kaxil kaxil Awaiting requested review from kaxil kaxil is a code owner

@XD-DENG XD-DENG Awaiting requested review from XD-DENG

Assignees

No one assigned

Labels

area:CLI area:dev-tools area:helm-chart Airflow Helm Chart area:providers area:Scheduler including HA (high availability) scheduler area:webserver Webserver related Issues kind:documentation provider:celery provider:cncf-kubernetes Kubernetes (k8s) provider related issues type:misc/internal Changelog: Misc changes that should appear in change log

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@potiuk @pankajkoti @eladkal @pierrejeambrun @o-nikolas @jedcunningham @vincbeck @ephraimbuddy