vincbeck commented on code in PR #33423:
URL: https://github.com/apache/airflow/pull/33423#discussion_r1297318297
##########
airflow/cli/cli_parser.py:
##########
@@ -59,13 +61,23 @@
"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
+ # Do not 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}
+# Check if sub-commands are defined twice, which could be an issue.
+if len(ALL_COMMANDS_DICT) < len(airflow_commands):
+ dup = {k for k, v in collections.Counter([c.name for c in
airflow_commands]).items() if v > 1}
+ raise CliConflictError(
+ f"The following CLI {len(dup)} command(s) are defined more than once:
{sorted(dup)}\n"
+ f"This can be due to the executor
'{ExecutorLoader.get_default_executor_name()}' "
+ f"redefining core airflow CLI commands."
Review Comment:
Do we want to have this message? In the future it might no longer be true
and other components such as auth managers could be able to also define their
own CLI command
##########
tests/cli/conftest.py:
##########
@@ -34,17 +36,14 @@
custom_executor_module.CustomCeleryKubernetesExecutor = type( # type: ignore
"CustomCeleryKubernetesExecutor",
(celery_kubernetes_executor.CeleryKubernetesExecutor,), {}
)
-custom_executor_module.CustomCeleryExecutor = type( # type: ignore
- "CustomLocalExecutor", (celery_executor.CeleryExecutor,), {}
-)
-custom_executor_module.CustomCeleryKubernetesExecutor = type( # type: ignore
- "CustomLocalKubernetesExecutor",
(celery_kubernetes_executor.CeleryKubernetesExecutor,), {}
+custom_executor_module.CustomLocalExecutor = type( # type: ignore
+ "CustomLocalExecutor", (local_executor.LocalExecutor,), {}
)
-custom_executor_module.CustomCeleryExecutor = type( # type: ignore
- "CustomKubernetesExecutor", (celery_executor.CeleryExecutor,), {}
+custom_executor_module.CustomLocalKubernetesExecutor = type( # type: ignore
+ "CustomLocalKubernetesExecutor",
(local_kubernetes_executor.LocalKubernetesExecutor,), {}
)
-custom_executor_module.CustomCeleryKubernetesExecutor = type( # type: ignore
- "CustomCeleryKubernetesExecutor",
(celery_kubernetes_executor.CeleryKubernetesExecutor,), {}
+custom_executor_module.CustomKubernetesExecutor = type( # type: ignore
+ "CustomKubernetesExecutor", (kubernetes_executor.KubernetesExecutor,), {}
Review Comment:
Nice!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
