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

Introduce mechanism to support multiple executor configuration #37635

Merged
merged 8 commits into from Mar 9, 2024

Conversation

o-nikolas
Copy link
Contributor

This commit delivers an isolated component of AIP-61: https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-61+Hybrid+Execution

It updates the executor loader logic to allow configuring multiple executors (and aliases per executor) in a list form (comma delimited) from airflow configuration; as well as associated methods for core Airflow code to load the default executor (first executor in the list) or any other configured executor.

Testing is included with the changes but not documentation, because AIP-61 is not yet complete. The code delivered is behind gating logic that will prohibit users from accidentally using it. User facing documentation will be delivered when the entire AIP-61 feature is fully completed.


^ 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 the area:Executors-core LocalExecutor & SequentialExecutor label Feb 22, 2024
@o-nikolas o-nikolas added the AIP-61 Hybrid executors label Feb 22, 2024
This commit delivers an isolated component of AIP-61:
https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-61+Hybrid+Execution

It updates the executor configuration logic to allow configuring multiple
executors (and aliases per executor) in a list form (comma delimited), as
well as associated methods for core Airflow code to load the default executor
(first executor in the list) or any other configured executor.

Testing is included with the changes but not documentation, because the
feature is not yet complete and is currently disabled. User facing
documentation will be delivered when the entire AIP-61 feature is
released.
@o-nikolas o-nikolas force-pushed the onikolas/aip-61/multiple_exec_config branch from 43b6b7a to de6dca1 Compare February 22, 2024 21:14
@o-nikolas o-nikolas marked this pull request as draft February 22, 2024 21:30
@o-nikolas
Copy link
Contributor Author

Converting to draft while I work on test failures ❤️

Executor data is now more aggressively cached and this was failing some tests
that are trying to override the executor conf. Moved the caches to the module
level and added some reload(executor_loader) calls to empty the caches for
certain tests that require it.
This unit test was broken to begin with and was working mostly by
chance. Updated the patch to be the correct method to mock and test
is passing again
@o-nikolas o-nikolas force-pushed the onikolas/aip-61/multiple_exec_config branch from d04cb0c to 68b920b Compare February 23, 2024 00:22
@o-nikolas o-nikolas marked this pull request as ready for review February 26, 2024 21:05
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

Nothing really surprising - good first approach to get configuration working for multiple executors. LGTM

Copy link
Contributor

@syedahsn syedahsn left a comment

Choose a reason for hiding this comment

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

LGTM, just left a comment asking for some clarification.

@o-nikolas
Copy link
Contributor Author

@potiuk

Do you have time to have another look and approve? I've made your suggestion and rebased and tests are green

@potiuk potiuk merged commit 93dfdf0 into apache:main Mar 9, 2024
56 checks passed
drajguru pushed a commit to drajguru/airflow that referenced this pull request Mar 14, 2024
…e#37635)

* Introduce mechanism to support multiple executor configuration

This commit delivers an isolated component of AIP-61:
https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-61+Hybrid+Execution

It updates the executor configuration logic to allow configuring multiple
executors (and aliases per executor) in a list form (comma delimited), as
well as associated methods for core Airflow code to load the default executor
(first executor in the list) or any other configured executor.

Testing is included with the changes but not documentation, because the
feature is not yet complete and is currently disabled. User facing
documentation will be delivered when the entire AIP-61 feature is
released.

* Move caches to module and use reloads to empty them in tests

Executor data is now more aggressively cached and this was failing some tests
that are trying to override the executor conf. Moved the caches to the module
level and added some reload(executor_loader) calls to empty the caches for
certain tests that require it.

* Fix TestBaseSensor::test_prepare_for_execution

This unit test was broken to begin with and was working mostly by
chance. Updated the patch to be the correct method to mock and test
is passing again

* More reloads

* Comment grammar fix

* New code needed a reload of executor loader module

* Fix unit test is-->are
@ephraimbuddy ephraimbuddy added the type:new-feature Changelog: New Features label Mar 14, 2024
howardyoo pushed a commit to howardyoo/airflow that referenced this pull request Mar 18, 2024
…e#37635)

* Introduce mechanism to support multiple executor configuration

This commit delivers an isolated component of AIP-61:
https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-61+Hybrid+Execution

It updates the executor configuration logic to allow configuring multiple
executors (and aliases per executor) in a list form (comma delimited), as
well as associated methods for core Airflow code to load the default executor
(first executor in the list) or any other configured executor.

Testing is included with the changes but not documentation, because the
feature is not yet complete and is currently disabled. User facing
documentation will be delivered when the entire AIP-61 feature is
released.

* Move caches to module and use reloads to empty them in tests

Executor data is now more aggressively cached and this was failing some tests
that are trying to override the executor conf. Moved the caches to the module
level and added some reload(executor_loader) calls to empty the caches for
certain tests that require it.

* Fix TestBaseSensor::test_prepare_for_execution

This unit test was broken to begin with and was working mostly by
chance. Updated the patch to be the correct method to mock and test
is passing again

* More reloads

* Comment grammar fix

* New code needed a reload of executor loader module

* Fix unit test is-->are
howardyoo pushed a commit to howardyoo/airflow that referenced this pull request Mar 31, 2024
…e#37635)

* Introduce mechanism to support multiple executor configuration

This commit delivers an isolated component of AIP-61:
https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-61+Hybrid+Execution

It updates the executor configuration logic to allow configuring multiple
executors (and aliases per executor) in a list form (comma delimited), as
well as associated methods for core Airflow code to load the default executor
(first executor in the list) or any other configured executor.

Testing is included with the changes but not documentation, because the
feature is not yet complete and is currently disabled. User facing
documentation will be delivered when the entire AIP-61 feature is
released.

* Move caches to module and use reloads to empty them in tests

Executor data is now more aggressively cached and this was failing some tests
that are trying to override the executor conf. Moved the caches to the module
level and added some reload(executor_loader) calls to empty the caches for
certain tests that require it.

* Fix TestBaseSensor::test_prepare_for_execution

This unit test was broken to begin with and was working mostly by
chance. Updated the patch to be the correct method to mock and test
is passing again

* More reloads

* Comment grammar fix

* New code needed a reload of executor loader module

* Fix unit test is-->are
utkarsharma2 pushed a commit to astronomer/airflow that referenced this pull request Apr 22, 2024
…e#37635)

* Introduce mechanism to support multiple executor configuration

This commit delivers an isolated component of AIP-61:
https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-61+Hybrid+Execution

It updates the executor configuration logic to allow configuring multiple
executors (and aliases per executor) in a list form (comma delimited), as
well as associated methods for core Airflow code to load the default executor
(first executor in the list) or any other configured executor.

Testing is included with the changes but not documentation, because the
feature is not yet complete and is currently disabled. User facing
documentation will be delivered when the entire AIP-61 feature is
released.

* Move caches to module and use reloads to empty them in tests

Executor data is now more aggressively cached and this was failing some tests
that are trying to override the executor conf. Moved the caches to the module
level and added some reload(executor_loader) calls to empty the caches for
certain tests that require it.

* Fix TestBaseSensor::test_prepare_for_execution

This unit test was broken to begin with and was working mostly by
chance. Updated the patch to be the correct method to mock and test
is passing again

* More reloads

* Comment grammar fix

* New code needed a reload of executor loader module

* Fix unit test is-->are
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AIP-61 Hybrid executors area:Executors-core LocalExecutor & SequentialExecutor type:new-feature Changelog: New Features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants