vandonr-amz commented on code in PR #33423:
URL: https://github.com/apache/airflow/pull/33423#discussion_r1297482474


##########
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:
   yes, we'll need to edit the message then, but in my opinion, we cannot crash 
the whole thing in the face of the user without giving them as much information 
as possible on why this crash is happening.
   
   Or maybe the only people who would encounter this error would be developers 
trying to add new commands and a vague message would be enough because they 
know what they just modified ?



-- 
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]

Reply via email to