bugraoz93 commented on code in PR #60193:
URL: https://github.com/apache/airflow/pull/60193#discussion_r2683504145


##########
airflow-ctl/src/airflowctl/ctl/cli_config.py:
##########
@@ -501,28 +501,28 @@ def _create_arg(
         arg_type: type | Callable,
         arg_help: str,
         arg_action: type[argparse.BooleanOptionalAction] | None,
-        arg_dest: str | None = None,
+        arg_dest: str | Any | None = None,
         arg_default: Any | None = None,
     ) -> Arg:
-        if not any(flag.startswith("-") for flag in arg_flags):
-            return Arg(
-                flags=arg_flags,
-                type=arg_type,
-                dest=_UNSET,
-                help=arg_help,
-                default=arg_default,
-                action=arg_action,
-            )
-        if arg_dest is None and len(arg_flags) > 0:
-            arg_dest = arg_flags[0].lstrip("-").replace("-", "_")
+        kwargs: dict[str, Any] = {"help": arg_help}
+
+        if arg_action is not None:

Review Comment:
   Rather simplified, you make it again complicated by making lots of 
conditions, while the `Arg()` method already accepts `None`. I think this 
method doesn't need a change at all to achieve this. 



##########
airflow-ctl/src/airflowctl/ctl/cli_config.py:
##########
@@ -584,13 +586,15 @@ def _create_args_map_from_operation(self):
                 for parameter_key, parameter_type in parameter.items():
                     if self._is_primitive_type(type_name=parameter_type):
                         is_bool = parameter_type == "bool"
+                        sanitized_key = 
self._sanitize_arg_parameter_key(parameter_key)
                         args.append(
                             self._create_arg(
-                                arg_flags=("--" + 
self._sanitize_arg_parameter_key(parameter_key),),
+                                arg_flags=("--" + sanitized_key,),

Review Comment:
   I think there is a misunderstanding of what we need to do with `--`. We can 
make it via without `--` and without dest, this is what we need and we should 
only set iff the field is required. In the current solution, we are a bit off 
track from what we need to achieve. If you run unit tests and update them, or 
even install the environment and start running `airflowctl` will see what I 
mean :)



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