o-nikolas commented on code in PR #33423:
URL: https://github.com/apache/airflow/pull/33423#discussion_r1297541587
##########
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:
> I dont disagree but at the same time, it might not be related at all to
the default executor and then point the wrong direction to the user
I think the language used in the message is already pretty cautious, it just
suggests that the executor _could_ be the issue:
> This can be due to the executor ...
We could try soften the language even more, but personally I think this is
okay for now
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
