Skip to content

Commit 9e286f2

Browse files
committed
Restore CLI commands from custom executors and auth managers
Signed-off-by: PoAn Yang <payang@apache.org>
1 parent 1952520 commit 9e286f2

2 files changed

Lines changed: 217 additions & 70 deletions

File tree

‎airflow-core/src/airflow/cli/cli_parser.py‎

Lines changed: 95 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@
4545
core_commands,
4646
)
4747
from airflow.cli.utils import CliConflictError
48-
from airflow.exceptions import AirflowException
48+
from airflow.configuration import conf
49+
from airflow.exceptions import AirflowConfigException, AirflowException
50+
from airflow.executors.executor_loader import ExecutorLoader
4951
from airflow.providers_manager import ProvidersManager
5052
from airflow.utils.helpers import partition
5153

@@ -59,6 +61,32 @@
5961

6062
log = logging.getLogger(__name__)
6163

64+
# Executors and auth managers shipped in airflow-core define no CLI commands of their own. Skipping them
65+
# by module prefix keeps the default configuration from importing any executor or auth manager class.
66+
_CORE_EXECUTORS_PACKAGE = "airflow.executors."
67+
_CORE_AUTH_MANAGERS_PACKAGE = "airflow.api_fastapi.auth.managers."
68+
69+
70+
def _exclude_registered_commands(
71+
commands: Iterable[CLICommand], registered_names: set[str], source: str
72+
) -> list[CLICommand]:
73+
"""
74+
Drop commands that a provider already registered through its ``cli`` section.
75+
76+
A custom class subclassing a provider executor or auth manager inherits its ``get_cli_commands`` and
77+
would otherwise re-register the same commands, which is reported as a conflict.
78+
"""
79+
result: list[CLICommand] = []
80+
for command in commands:
81+
if command.name in registered_names:
82+
log.debug(
83+
"Skipping CLI command '%s' from %s: already registered by a provider.", command.name, source
84+
)
85+
continue
86+
result.append(command)
87+
return result
88+
89+
6290
# AIRFLOW_PACKAGE_NAME is set when generating docs and we don't want to load provider commands when generating airflow-core CLI docs
6391
if not os.environ.get("AIRFLOW_PACKAGE_NAME", None):
6492
providers_manager = ProvidersManager()
@@ -76,6 +104,11 @@
76104
log.warning("Failed to load CLI commands from providers: %s", e)
77105
# do not re-raise for the same reason as above
78106

107+
# Commands registered through a provider "cli" section take precedence over the compat loading below
108+
provider_command_names = {command.name for command in airflow_commands} - {
109+
command.name for command in core_commands
110+
}
111+
79112
WARNING_TEMPLATE = """
80113
Please define the 'cli' section in the 'get_provider_info' for custom {component} to avoid this warning.
81114
For community providers, please update to the version that support 'cli' section.
@@ -84,10 +117,10 @@
84117
Providers with {component} missing 'cli' section in 'get_provider_info': {not_defined_cli_dict}
85118
"""
86119

87-
# compat loading for older providers that define get_cli_commands methods on Executors
120+
# compat loading for executors that do not register CLI commands through a provider "cli" section:
121+
# older providers, and custom executors configured by module path without being packaged as a provider
88122
try:
89-
# if there is any executor_provider not in cli_provider, we have to do compat loading
90-
# we use without check to avoid actual loading in this check
123+
# warn about providers that still rely on compat loading; "without check" avoids importing them here
91124
executors_not_defined_cli = {
92125
executor_name: executor_provider
93126
for executor_name, executor_provider in providers_manager.executor_without_check
@@ -99,40 +132,48 @@
99132
component="executors", not_defined_cli_dict=str(executors_not_defined_cli)
100133
)
101134
)
102-
from airflow.executors.executor_loader import ExecutorLoader
103-
104-
for executor_name in ExecutorLoader.get_executor_names(validate_teams=False):
105-
# Skip if the executor already has CLI commands defined via the 'cli' section in provider.yaml
106-
if executor_name.module_path not in executors_not_defined_cli:
107-
log.debug(
108-
"Skipping loading for '%s' as it is defined in 'cli' section.",
109-
executor_name.module_path,
110-
)
111-
continue
112-
113-
try:
114-
executor, _ = ExecutorLoader.import_executor_cls(executor_name)
115-
airflow_commands.extend(executor.get_cli_commands())
116-
except Exception:
117-
log.exception("Failed to load CLI commands from executor: %s", executor_name)
118-
log.error(
119-
"Ensure all dependencies are met and try again. If using a Celery based executor install "
120-
"a 3.3.0+ version of the Celery provider. If using a Kubernetes executor, install a "
121-
"7.4.0+ version of the CNCF provider"
135+
executors_defined_cli = {
136+
executor_name
137+
for executor_name, executor_provider in providers_manager.executor_without_check
138+
if executor_provider in providers_manager.cli_command_providers
139+
}
140+
141+
for executor_name in ExecutorLoader.get_executor_names(validate_teams=False):
142+
if executor_name.module_path in executors_defined_cli or executor_name.module_path.startswith(
143+
_CORE_EXECUTORS_PACKAGE
144+
):
145+
log.debug(
146+
"Skipping loading for '%s' as its CLI commands are registered elsewhere.",
147+
executor_name.module_path,
148+
)
149+
continue
150+
151+
try:
152+
executor, _ = ExecutorLoader.import_executor_cls(executor_name)
153+
airflow_commands.extend(
154+
_exclude_registered_commands(
155+
executor.get_cli_commands(), provider_command_names, executor_name.module_path
122156
)
123-
# Do not re-raise the exception since we want the CLI to still function for
124-
# other commands.
157+
)
158+
except Exception:
159+
log.exception("Failed to load CLI commands from executor: %s", executor_name)
160+
log.error(
161+
"Ensure all dependencies are met and try again. If using a Celery based executor install "
162+
"a 3.3.0+ version of the Celery provider. If using a Kubernetes executor, install a "
163+
"7.4.0+ version of the CNCF provider"
164+
)
165+
# Do not re-raise the exception since we want the CLI to still function for
166+
# other commands.
125167

126168
except Exception as e:
127169
log.warning(
128170
"Failed to load CLI commands from executors that didn't define `get_cli_commands` in `.cli.definition`: %s",
129171
e,
130172
)
131173

132-
# compat loading for older providers that define get_cli_commands methods on AuthManagers
174+
# compat loading for auth managers, following the same rules as for executors
133175
try:
134-
# if there is any auth_manager not in cli_provider, we have to do compat loading
135-
# we use without check to avoid actual loading in this check
176+
# warn about providers that still rely on compat loading; "without check" avoids importing them here
136177
auth_managers_not_defined_cli = {
137178
auth_manager_name: auth_manager_provider
138179
for auth_manager_name, auth_manager_provider in providers_manager.auth_manager_without_check
@@ -144,27 +185,35 @@
144185
component="auth manager", not_defined_cli_dict=str(auth_managers_not_defined_cli)
145186
)
146187
)
188+
auth_managers_defined_cli = {
189+
auth_manager_name
190+
for auth_manager_name, auth_manager_provider in providers_manager.auth_manager_without_check
191+
if auth_manager_provider in providers_manager.cli_command_providers
192+
}
147193

148-
from airflow.configuration import conf
149-
from airflow.exceptions import AirflowConfigException
194+
auth_manager_cls_path = conf.get(section="core", key="auth_manager")
150195

151-
auth_manager_cls_path = conf.get(section="core", key="auth_manager")
196+
if not auth_manager_cls_path:
197+
raise AirflowConfigException(
198+
"No auth manager defined in the config. Please specify one using section/key [core/auth_manager]."
199+
)
152200

153-
if not auth_manager_cls_path:
154-
raise AirflowConfigException(
155-
"No auth manager defined in the config. Please specify one using section/key [core/auth_manager]."
201+
if auth_manager_cls_path not in auth_managers_defined_cli and not auth_manager_cls_path.startswith(
202+
_CORE_AUTH_MANAGERS_PACKAGE
203+
):
204+
try:
205+
auth_manager_cls = import_string(auth_manager_cls_path)
206+
auth_manager = auth_manager_cls()
207+
airflow_commands.extend(
208+
_exclude_registered_commands(
209+
auth_manager.get_cli_commands(), provider_command_names, auth_manager_cls_path
210+
)
156211
)
157-
158-
if auth_manager_cls_path in auth_managers_not_defined_cli:
159-
try:
160-
auth_manager_cls = import_string(auth_manager_cls_path)
161-
auth_manager = auth_manager_cls()
162-
airflow_commands.extend(auth_manager.get_cli_commands())
163-
except Exception:
164-
log.exception("Failed to load CLI commands from auth manager: %s", auth_manager_cls)
165-
log.error("Ensure all dependencies are met and try again.")
166-
# Do not re-raise the exception since we want the CLI to still function for
167-
# other commands.
212+
except Exception:
213+
log.exception("Failed to load CLI commands from auth manager: %s", auth_manager_cls_path)
214+
log.error("Ensure all dependencies are met and try again.")
215+
# Do not re-raise the exception since we want the CLI to still function for
216+
# other commands.
168217
except Exception as e:
169218
log.warning(
170219
"Failed to load CLI commands from auth managers that didn't define `get_cli_commands` in `.cli.definition`: %s",

0 commit comments

Comments
 (0)