-
Notifications
You must be signed in to change notification settings - Fork 16.4k
[AIP-51] Executors vending CLI commands #29055
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
Merged
potiuk
merged 6 commits into
apache:main
from
aws-mwaa:onikolas/aip-51/executor_cli_vending
Jul 29, 2023
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
78b6b5f
Base implementation of executor vending cli commands
o-nikolas 5965ed4
Move existing Celery and Kubernetes cli commands to their module
o-nikolas ea9416b
Updates required after celery executor move to providers
o-nikolas d4ca235
Final touches and updates from PR
o-nikolas d223ec5
Updates from PR feedback
o-nikolas a42536c
Update error message when failing to import executor CLI commands
o-nikolas File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,7 @@ | |
| from __future__ import annotations | ||
|
|
||
| import argparse | ||
| import logging | ||
| from argparse import Action | ||
| from functools import lru_cache | ||
| from typing import Iterable | ||
|
|
@@ -41,10 +42,27 @@ | |
| core_commands, | ||
| ) | ||
| from airflow.exceptions import AirflowException | ||
| from airflow.executors.executor_loader import ExecutorLoader | ||
| from airflow.utils.helpers import partition | ||
|
|
||
| airflow_commands = core_commands | ||
|
|
||
| log = logging.getLogger(__name__) | ||
| try: | ||
| executor, _ = ExecutorLoader.import_default_executor_cls(validate=False) | ||
| airflow_commands.extend(executor.get_cli_commands()) | ||
| except Exception: | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Under no circumstances should the executor commands fail the parsing process. So except any Exception here. |
||
| executor_name = ExecutorLoader.get_default_executor_name() | ||
| log.exception("Failed to load CLI commands from executor: %s", executor_name) | ||
| log.error( | ||
| "Ensure all dependencies are met and try again. If using a Celery based executor install " | ||
| "a 3.3.0+ version of the Celery provider. If using a Kubernetes executor, install a " | ||
| "7.4.0+ version of the CNCF provider" | ||
| ) | ||
| # Do no re-raise the exception since we want the CLI to still function for | ||
| # other commands. | ||
|
|
||
|
|
||
| ALL_COMMANDS_DICT: dict[str, CLICommand] = {sp.name: sp for sp in airflow_commands} | ||
|
|
||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of this was an attempt to handle the situation of using CLI commands with missing deps. Now only the configured executor commands are available and we assume that the dependencies for that are installed (otherwise the user has much bigger issues to worry about).