[GitHub] [airflow] turbaszek commented on a change in pull request #7801: Remove arg lookup map in CLIFactory

[GitBox <gi...@apache.org>]

turbaszek commented on a change in pull request #7801: Remove arg lookup map in CLIFactory
URL: https://github.com/apache/airflow/pull/7801#discussion_r396024271
 
 

 ##########
 File path: airflow/bin/cli.py
 ##########
 @@ -1082,35 +1155,46 @@ def get_long_option(arg):
             """
             Get long option from Arg.flags
             """
-            return cls.args[arg].flags[0] if len(cls.args[arg].flags) == 1 else cls.args[arg].flags[1]
-        positional, optional = partition(lambda x: cls.args[x].flags[0].startswith("-"), args)
+            return arg.flags[0] if len(arg.flags) == 1 else arg.flags[1]
+        positional, optional = partition(lambda x: x.flags[0].startswith("-"), args)
         yield from positional
         yield from sorted(optional, key=lambda x: get_long_option(x).lower())
 
     @classmethod
-    def _add_subcommand(cls, subparsers, sub):
-        dag_parser = False
+    def _add_command(cls, subparsers, sub):
         sub_proc = subparsers.add_parser(
-            sub.get('name') or sub['func'].__name__, help=sub['help']  # type: ignore
+            sub['name'], help=sub['help']
         )
         sub_proc.formatter_class = RawTextHelpFormatter
 
-        subcommands = sub.get('subcommands', [])
-        if subcommands:
-            sub_subparsers = sub_proc.add_subparsers(dest='subcommand')
-            sub_subparsers.required = True
-            for command in sorted(subcommands, key=lambda x: x['name']):
-                cls._add_subcommand(sub_subparsers, command)
+        if 'subcommands' in sub:
+            cls._add_subcommaand(sub, sub_proc)
+        elif 'func' in sub:
+            cls._add_action_command(sub, sub_proc)
         else:
-            for arg in cls.sort_args(sub['args']):
-                if 'dag_id' in arg and dag_parser:
-                    continue
-                arg = cls.args[arg]
-                kwargs = {
-                    f: v
-                    for f, v in vars(arg).items() if f != 'flags' and v}
-                sub_proc.add_argument(*arg.flags, **kwargs)
-            sub_proc.set_defaults(func=sub['func'])
+            raise AirflowException("Invalid command definition.")
+
+    @classmethod
+    def _add_action_command(cls, sub, sub_proc):
+        dag_parser = False
+
+        for arg in cls.sort_args(sub['args']):
+            if 'dag_id' in arg.flags and dag_parser:
+                continue
+            kwargs = {
+                k: v for k, v in vars(arg).items() if k != 'flags' and v
+            }
+            sub_proc.add_argument(*arg.flags, **kwargs)
+        sub_proc.set_defaults(func=sub['func'])
 
 Review comment:
   What is the purpose of `dag_parser`? If I'm not mistaken it's always equal to `False` so the first condition from the loop is useless. 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services