From 25caf7f1d9412c44f7f8152a27a6227e9f1406bf Mon Sep 17 00:00:00 2001 From: Fury0508 Date: Tue, 6 Jan 2026 23:33:21 +0000 Subject: [PATCH 01/16] Implement positional parameters for required CLI args in airflowctl Related to #60142 - Modified _create_arg() to accept is_required parameter - Required non-boolean parameters are now positional (no -- prefix) - Optional parameters remain as options with -- prefix and dest - Uses field_type.is_required() to detect required Pydantic fields - Positional args use _UNSET for dest to avoid argparse error --- airflow-ctl/src/airflowctl/ctl/cli_config.py | 24 ++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/airflow-ctl/src/airflowctl/ctl/cli_config.py b/airflow-ctl/src/airflowctl/ctl/cli_config.py index 466ee671b61b2..c964191f827fe 100755 --- a/airflow-ctl/src/airflowctl/ctl/cli_config.py +++ b/airflow-ctl/src/airflowctl/ctl/cli_config.py @@ -522,7 +522,28 @@ def _create_arg( arg_action: argparse.BooleanOptionalAction | None, arg_dest: str | None = None, arg_default: Any | None = None, + is_required: bool = False, # NEW PARAMETER ) -> Arg: + # Boolean flags should always remain optional (with --) + # Required non-boolean fields should be positional + should_be_positional = is_required and arg_action is None + + if should_be_positional: + # Make it positional - remove '--' prefix + clean_flags = tuple(flag.lstrip("-").replace("-", "_") for flag in arg_flags) + return Arg( + flags=clean_flags, + type=arg_type, + dest=_UNSET, # Use _UNSET so dest is not added to kwargs + help=arg_help, + default=arg_default, + action=arg_action, + ) + # Keep as optional - add dest parameter if not provided + if arg_dest is None and len(arg_flags) > 0: + # Generate dest from flag name + arg_dest = arg_flags[0].lstrip("-").replace("-", "_") + return Arg( flags=arg_flags, type=arg_type, @@ -545,6 +566,7 @@ def _create_arg_for_non_primitive_type( for field, field_type in parameter_type_map.model_fields.items(): if field in self.excluded_parameters: continue + is_required = field_type.is_required() self.datamodels_extended_map[parameter_type].append(field) if type(field_type.annotation) is type: commands.append( @@ -554,6 +576,7 @@ def _create_arg_for_non_primitive_type( arg_action=argparse.BooleanOptionalAction if field_type.annotation is bool else None, # type: ignore arg_help=f"{field} for {parameter_key} operation", arg_default=False if field_type.annotation is bool else None, + is_required=is_required, ) ) else: @@ -569,6 +592,7 @@ def _create_arg_for_non_primitive_type( arg_action=argparse.BooleanOptionalAction if annotation is bool else None, # type: ignore arg_help=f"{field} for {parameter_key} operation", arg_default=False if annotation is bool else None, + is_required=is_required, ) ) return commands From b64e4a0518dc40fc76d9fac7002f0fba2c55a13f Mon Sep 17 00:00:00 2001 From: Fury0508 Date: Wed, 7 Jan 2026 23:26:08 +0000 Subject: [PATCH 02/16] Address review feedback: simplify positional parameter logic and add tests - Remove unnecessary comments - Simplify logic using one-liner conditional for flag determination - Add unit test for positional vs optional argument handling - _create_arg now uses simple flag checking instead of is_required parameter --- airflow-ctl/src/airflowctl/ctl/cli_config.py | 40 +++---- .../tests/airflow_ctl/ctl/test_cli_config.py | 109 +++++++++++------- 2 files changed, 84 insertions(+), 65 deletions(-) diff --git a/airflow-ctl/src/airflowctl/ctl/cli_config.py b/airflow-ctl/src/airflowctl/ctl/cli_config.py index c964191f827fe..c447921806a7b 100755 --- a/airflow-ctl/src/airflowctl/ctl/cli_config.py +++ b/airflow-ctl/src/airflowctl/ctl/cli_config.py @@ -519,29 +519,20 @@ def _create_arg( arg_flags: tuple, arg_type: type | Callable, arg_help: str, - arg_action: argparse.BooleanOptionalAction | None, + arg_action: type[argparse.BooleanOptionalAction] | None, arg_dest: str | None = None, arg_default: Any | None = None, - is_required: bool = False, # NEW PARAMETER ) -> Arg: - # Boolean flags should always remain optional (with --) - # Required non-boolean fields should be positional - should_be_positional = is_required and arg_action is None - - if should_be_positional: - # Make it positional - remove '--' prefix - clean_flags = tuple(flag.lstrip("-").replace("-", "_") for flag in arg_flags) + if not any(flag.startswith("-") for flag in arg_flags): return Arg( - flags=clean_flags, + flags=arg_flags, type=arg_type, - dest=_UNSET, # Use _UNSET so dest is not added to kwargs + dest=_UNSET, help=arg_help, default=arg_default, action=arg_action, ) - # Keep as optional - add dest parameter if not provided if arg_dest is None and len(arg_flags) > 0: - # Generate dest from flag name arg_dest = arg_flags[0].lstrip("-").replace("-", "_") return Arg( @@ -566,17 +557,22 @@ def _create_arg_for_non_primitive_type( for field, field_type in parameter_type_map.model_fields.items(): if field in self.excluded_parameters: continue + is_required = field_type.is_required() + sanitized_field = self._sanitize_arg_parameter_key(field) self.datamodels_extended_map[parameter_type].append(field) + if type(field_type.annotation) is type: + is_bool = field_type.annotation is bool + arg_flags = (sanitized_field,) if is_required and not is_bool else ("--" + sanitized_field,) + commands.append( self._create_arg( - arg_flags=("--" + self._sanitize_arg_parameter_key(field),), + arg_flags=arg_flags, arg_type=self._python_type_from_string(field_type.annotation), - arg_action=argparse.BooleanOptionalAction if field_type.annotation is bool else None, # type: ignore + arg_action=argparse.BooleanOptionalAction if is_bool else None, arg_help=f"{field} for {parameter_key} operation", - arg_default=False if field_type.annotation is bool else None, - is_required=is_required, + arg_default=False if is_bool else None, ) ) else: @@ -585,14 +581,16 @@ def _create_arg_for_non_primitive_type( except AttributeError: annotation = field_type.annotation + is_bool = annotation is bool + arg_flags = (sanitized_field,) if is_required and not is_bool else ("--" + sanitized_field,) + commands.append( self._create_arg( - arg_flags=("--" + self._sanitize_arg_parameter_key(field),), + arg_flags=arg_flags, arg_type=self._python_type_from_string(annotation), - arg_action=argparse.BooleanOptionalAction if annotation is bool else None, # type: ignore + arg_action=argparse.BooleanOptionalAction if is_bool else None, arg_help=f"{field} for {parameter_key} operation", - arg_default=False if annotation is bool else None, - is_required=is_required, + arg_default=False if is_bool else None, ) ) return commands diff --git a/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py b/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py index e0278cd7c5348..77005e032de5f 100644 --- a/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py +++ b/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py @@ -19,6 +19,7 @@ import argparse from argparse import BooleanOptionalAction +from datetime import datetime from textwrap import dedent import httpx @@ -59,7 +60,7 @@ def no_op(): def test_args_create(): return [ ( - "--dag-id", + "dag-id", { "help": "dag_id for backfill operation", "action": None, @@ -69,22 +70,22 @@ def test_args_create(): }, ), ( - "--from-date", + "from-date", { "help": "from_date for backfill operation", "action": None, "default": None, - "type": str, + "type": datetime, "dest": None, }, ), ( - "--to-date", + "to-date", { "help": "to_date for backfill operation", "action": None, "default": None, - "type": str, + "type": datetime, "dest": None, }, ), @@ -540,52 +541,52 @@ def test_add_auth_token_to_all_commands(self, no_op_method): assert command.args == expected_command_2_args def test_trigger_dag_run_defaults_logical_date_to_now(self): - """Test that trigger command defaults logical_date to now when not provided.""" - from datetime import datetime, timezone - - from airflowctl.api.datamodels.generated import TriggerDAGRunPostBody - - # Simulate the logic in _get_func from cli_config.py - # This is the actual code path that runs when user doesn't provide --logical-date - - # Step 1: Simulate CLI args being parsed (logical_date=None) - method_params = { - "trigger_dag_run": { - "dag_run_id": None, - "data_interval_start": None, - "data_interval_end": None, - "logical_date": None, # User did not provide --logical-date - "run_after": None, - "conf": None, - "note": None, - "partition_key": None, + """Test that trigger command defaults logical_date to now when not provided.""" + from datetime import datetime, timezone + + from airflowctl.api.datamodels.generated import TriggerDAGRunPostBody + + # Simulate the logic in _get_func from cli_config.py + # This is the actual code path that runs when user doesn't provide --logical-date + + # Step 1: Simulate CLI args being parsed (logical_date=None) + method_params = { + "trigger_dag_run": { + "dag_run_id": None, + "data_interval_start": None, + "data_interval_end": None, + "logical_date": None, # User did not provide --logical-date + "run_after": None, + "conf": None, + "note": None, + "partition_key": None, + } } - } - # Step 2: Apply the defaulting logic (from cli_config.py lines 622-630) - datamodel = TriggerDAGRunPostBody - datamodel_param_name = "trigger_dag_run" + # Step 2: Apply the defaulting logic (from cli_config.py lines 622-630) + datamodel = TriggerDAGRunPostBody + datamodel_param_name = "trigger_dag_run" - if ( - datamodel.__name__ == "TriggerDAGRunPostBody" - and "logical_date" in method_params[datamodel_param_name] - and method_params[datamodel_param_name]["logical_date"] is None - ): - method_params[datamodel_param_name]["logical_date"] = datetime.now(timezone.utc) + if ( + datamodel.__name__ == "TriggerDAGRunPostBody" + and "logical_date" in method_params[datamodel_param_name] + and method_params[datamodel_param_name]["logical_date"] is None + ): + method_params[datamodel_param_name]["logical_date"] = datetime.now(timezone.utc) - # Step 3: Create the Pydantic model (what happens in the actual code) - trigger_body = datamodel.model_validate(method_params[datamodel_param_name]) + # Step 3: Create the Pydantic model (what happens in the actual code) + trigger_body = datamodel.model_validate(method_params[datamodel_param_name]) - # Step 4: Verify logical_date was set to now - assert trigger_body.logical_date is not None, "logical_date should be defaulted to now" - assert isinstance(trigger_body.logical_date, datetime) + # Step 4: Verify logical_date was set to now + assert trigger_body.logical_date is not None, "logical_date should be defaulted to now" + assert isinstance(trigger_body.logical_date, datetime) - # Verify it's close to current time (within 5 seconds) - time_diff = abs((datetime.now(timezone.utc) - trigger_body.logical_date).total_seconds()) - assert time_diff < 5, f"logical_date should be close to now, but diff is {time_diff} seconds" + # Verify it's close to current time (within 5 seconds) + time_diff = abs((datetime.now(timezone.utc) - trigger_body.logical_date).total_seconds()) + assert time_diff < 5, f"logical_date should be close to now, but diff is {time_diff} seconds" - # Also verify timezone is UTC - assert trigger_body.logical_date.tzinfo is not None, "logical_date should have timezone info" + # Also verify timezone is UTC + assert trigger_body.logical_date.tzinfo is not None, "logical_date should have timezone info" def test_apply_datamodel_defaults_trigger_dag_run_with_none(self): """Test _apply_datamodel_defaults sets logical_date to now when None for TriggerDAGRunPostBody.""" @@ -669,3 +670,23 @@ def test_help_texts_used_for_auto_generated_commands(self, group_name, subcomman "Help message should match the help_text.yaml" ) return + def test_positional_args(self): + """Test that required parameters are created as positional arguments.""" + command_factory = CommandFactory(file_path="") + + positional_arg = command_factory._create_arg( + arg_flags=("connection_id",), + arg_type=str, + arg_help="Connection ID", + arg_action=None, + ) + assert positional_arg.flags[0] == "connection_id" + + optional_arg = command_factory._create_arg( + arg_flags=("--description",), + arg_type=str, + arg_help="Description", + arg_action=None, + ) + assert optional_arg.flags[0] == "--description" + From f5f1e28b0238778a1dae45355e5d4710578d764e Mon Sep 17 00:00:00 2001 From: Fury0508 Date: Thu, 8 Jan 2026 23:46:19 +0000 Subject: [PATCH 03/16] Fix test fixture: use positional args for required fields --- airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py b/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py index 77005e032de5f..bf6b76d56f691 100644 --- a/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py +++ b/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py @@ -19,7 +19,6 @@ import argparse from argparse import BooleanOptionalAction -from datetime import datetime from textwrap import dedent import httpx @@ -75,7 +74,7 @@ def test_args_create(): "help": "from_date for backfill operation", "action": None, "default": None, - "type": datetime, + "type": str, "dest": None, }, ), @@ -85,7 +84,7 @@ def test_args_create(): "help": "to_date for backfill operation", "action": None, "default": None, - "type": datetime, + "type": str, "dest": None, }, ), From 03dab165d45c1bc4090dec8690cf263b0e88045a Mon Sep 17 00:00:00 2001 From: Fury0508 Date: Sat, 10 Jan 2026 23:20:32 +0000 Subject: [PATCH 04/16] Simplify positional parameter logic by moving decision to caller - Remove if/else branching from _create_arg method - Caller now determines both arg_flags and arg_dest - _create_arg is now a simple builder with no conditional logic - Updated all three call sites to pass arg_dest parameter This addresses maintainer feedback to make the code more modular by having the caller manage the logic rather than the method. --- airflow-ctl/src/airflowctl/ctl/cli_config.py | 46 +++++++++++--------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/airflow-ctl/src/airflowctl/ctl/cli_config.py b/airflow-ctl/src/airflowctl/ctl/cli_config.py index c447921806a7b..ed66e003dbcbf 100755 --- a/airflow-ctl/src/airflowctl/ctl/cli_config.py +++ b/airflow-ctl/src/airflowctl/ctl/cli_config.py @@ -520,28 +520,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: + kwargs["action"] = arg_action + else: + kwargs["type"] = arg_type + + if arg_default is not None: + kwargs["default"] = arg_default + + if arg_dest is None: + final_dest = arg_flags[0].lstrip("-").replace("-", "_") + else: + final_dest = arg_dest return Arg( flags=arg_flags, - type=arg_type, - dest=arg_dest, - help=arg_help, - default=arg_default, - action=arg_action, + dest=final_dest, + **kwargs, ) def _create_arg_for_non_primitive_type( @@ -565,7 +565,7 @@ def _create_arg_for_non_primitive_type( if type(field_type.annotation) is type: is_bool = field_type.annotation is bool arg_flags = (sanitized_field,) if is_required and not is_bool else ("--" + sanitized_field,) - + arg_dest = _UNSET if is_required and not is_bool else sanitized_field commands.append( self._create_arg( arg_flags=arg_flags, @@ -573,6 +573,7 @@ def _create_arg_for_non_primitive_type( arg_action=argparse.BooleanOptionalAction if is_bool else None, arg_help=f"{field} for {parameter_key} operation", arg_default=False if is_bool else None, + arg_dest=arg_dest, ) ) else: @@ -583,7 +584,7 @@ def _create_arg_for_non_primitive_type( is_bool = annotation is bool arg_flags = (sanitized_field,) if is_required and not is_bool else ("--" + sanitized_field,) - + arg_dest = _UNSET if is_required and not is_bool else sanitized_field commands.append( self._create_arg( arg_flags=arg_flags, @@ -591,6 +592,7 @@ def _create_arg_for_non_primitive_type( arg_action=argparse.BooleanOptionalAction if is_bool else None, arg_help=f"{field} for {parameter_key} operation", arg_default=False if is_bool else None, + arg_dest=arg_dest, ) ) return commands @@ -604,13 +606,15 @@ def _create_args_map_from_operation(self): if self._is_primitive_type(type_name=parameter_type): base_parameter_type = parameter_type.replace(" | None", "").strip() is_bool = base_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,), arg_type=self._python_type_from_string(parameter_type), arg_action=argparse.BooleanOptionalAction if is_bool else None, arg_help=f"{parameter_key} for {operation.get('name')} operation in {operation.get('parent').name}", - arg_default=None, + arg_default=False if is_bool else None, + arg_dest=sanitized_key, ) ) else: From 0d6bb69b9fbb5ac57a95c2cf95153678a16ede5b Mon Sep 17 00:00:00 2001 From: Fury0508 Date: Mon, 12 Jan 2026 21:33:06 +0000 Subject: [PATCH 05/16] Implement positional parameters for required CLI arguments - Modified arg_flags generation to omit -- prefix for required non-boolean parameters - Changed _create_arg defaults to use _UNSET instead of None - Caller determines flags based on is_required, _create_arg remains a simple pass-through method - Required non-boolean parameters now appear as positional arguments in CLI --- airflow-ctl/src/airflowctl/ctl/cli_config.py | 31 +++++--------------- 1 file changed, 7 insertions(+), 24 deletions(-) diff --git a/airflow-ctl/src/airflowctl/ctl/cli_config.py b/airflow-ctl/src/airflowctl/ctl/cli_config.py index ed66e003dbcbf..4c4b277b04b6b 100755 --- a/airflow-ctl/src/airflowctl/ctl/cli_config.py +++ b/airflow-ctl/src/airflowctl/ctl/cli_config.py @@ -520,28 +520,16 @@ def _create_arg( arg_type: type | Callable, arg_help: str, arg_action: type[argparse.BooleanOptionalAction] | None, - arg_dest: str | Any | None = None, - arg_default: Any | None = None, + arg_dest=_UNSET, + arg_default=_UNSET, ) -> Arg: - kwargs: dict[str, Any] = {"help": arg_help} - - if arg_action is not None: - kwargs["action"] = arg_action - else: - kwargs["type"] = arg_type - - if arg_default is not None: - kwargs["default"] = arg_default - - if arg_dest is None: - final_dest = arg_flags[0].lstrip("-").replace("-", "_") - else: - final_dest = arg_dest - return Arg( flags=arg_flags, - dest=final_dest, - **kwargs, + type=arg_type, + dest=arg_dest, + help=arg_help, + default=arg_default, + action=arg_action, ) def _create_arg_for_non_primitive_type( @@ -565,7 +553,6 @@ def _create_arg_for_non_primitive_type( if type(field_type.annotation) is type: is_bool = field_type.annotation is bool arg_flags = (sanitized_field,) if is_required and not is_bool else ("--" + sanitized_field,) - arg_dest = _UNSET if is_required and not is_bool else sanitized_field commands.append( self._create_arg( arg_flags=arg_flags, @@ -573,7 +560,6 @@ def _create_arg_for_non_primitive_type( arg_action=argparse.BooleanOptionalAction if is_bool else None, arg_help=f"{field} for {parameter_key} operation", arg_default=False if is_bool else None, - arg_dest=arg_dest, ) ) else: @@ -584,7 +570,6 @@ def _create_arg_for_non_primitive_type( is_bool = annotation is bool arg_flags = (sanitized_field,) if is_required and not is_bool else ("--" + sanitized_field,) - arg_dest = _UNSET if is_required and not is_bool else sanitized_field commands.append( self._create_arg( arg_flags=arg_flags, @@ -592,7 +577,6 @@ def _create_arg_for_non_primitive_type( arg_action=argparse.BooleanOptionalAction if is_bool else None, arg_help=f"{field} for {parameter_key} operation", arg_default=False if is_bool else None, - arg_dest=arg_dest, ) ) return commands @@ -614,7 +598,6 @@ def _create_args_map_from_operation(self): arg_action=argparse.BooleanOptionalAction if is_bool else None, arg_help=f"{parameter_key} for {operation.get('name')} operation in {operation.get('parent').name}", arg_default=False if is_bool else None, - arg_dest=sanitized_key, ) ) else: From e2f276c569c4cf284c30975e828af6b6f188c52a Mon Sep 17 00:00:00 2001 From: Fury0508 Date: Mon, 19 Jan 2026 10:06:52 +0000 Subject: [PATCH 06/16] Update test to skip dest check for positional args --- airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py b/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py index bf6b76d56f691..6b1b11eacd64d 100644 --- a/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py +++ b/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py @@ -275,7 +275,8 @@ def delete(self, backfill_id: str) -> ServerResponseError | None: assert arg.kwargs["action"] == test_arg[1]["action"] assert arg.kwargs["default"] == test_arg[1]["default"] assert arg.kwargs["type"] == test_arg[1]["type"] - assert arg.kwargs["dest"] == test_arg[1]["dest"] + if "dest" in test_arg[1]: + assert arg.kwargs.get("dest") == test_arg[1]["dest"] print(arg.flags) elif sub_command.name == "list": for arg, test_arg in zip(sub_command.args, test_args_list): From bdf466a5ab62ef44e5c17cd712fd786d96071cc3 Mon Sep 17 00:00:00 2001 From: Fury0508 Date: Wed, 21 Jan 2026 12:46:05 +0000 Subject: [PATCH 07/16] Update integration tests to use positional args syntax --- .../airflowctl_tests/test_airflowctl_commands.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py b/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py index de07d0d3751a9..66a6108476b6d 100644 --- a/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py +++ b/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py @@ -54,7 +54,7 @@ def date_param(): "auth list-envs", # Assets commands "assets list", - "assets get --asset-id=1", + "assets create-event 1", "assets create-event --asset-id=1", # Backfill commands "backfill list", @@ -63,13 +63,13 @@ def date_param(): "config list", "config lint", # Connections commands - "connections create --connection-id=test_con --conn-type=mysql --password=TEST_PASS -o json", + "connections create test_con mysql --password=TEST_PASS -o json", "connections list", "connections list -o yaml", "connections list -o table", "connections get --conn-id=test_con", "connections get --conn-id=test_con -o json", - "connections update --connection-id=test_con --conn-type=postgres", + "connections update test_con postgres", "connections import tests/airflowctl_tests/fixtures/test_connections.json", "connections delete --conn-id=test_con", "connections delete --conn-id=test_import_conn", @@ -102,7 +102,7 @@ def date_param(): # Jobs commands "jobs list", # Pools commands - "pools create --name=test_pool --slots=5", + "pools create test_pool 5", "pools list", "pools get --pool-name=test_pool", "pools get --pool-name=test_pool -o yaml", @@ -114,11 +114,11 @@ def date_param(): # Providers commands "providers list", # Variables commands - "variables create --key=test_key --value=test_value", + "variables create test_key test_value", "variables list", "variables get --variable-key=test_key", "variables get --variable-key=test_key -o table", - "variables update --key=test_key --value=updated_value", + "variables update test_key updated_value", "variables import tests/airflowctl_tests/fixtures/test_variables.json", "variables delete --variable-key=test_key", "variables delete --variable-key=test_import_var", From a3d841191d6ed6be816664d77a2679b7a80b38b5 Mon Sep 17 00:00:00 2001 From: Fury0508 Date: Tue, 3 Feb 2026 21:22:38 +0530 Subject: [PATCH 08/16] Add integration test for 'assets get' command - Adds test coverage for 'assets get 1' to test_airflowctl_commands.py - Resolves pre-commit hook failure about missing test coverage --- .../tests/airflowctl_tests/test_airflowctl_commands.py | 1 + 1 file changed, 1 insertion(+) diff --git a/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py b/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py index 66a6108476b6d..5ad48f3bbc605 100644 --- a/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py +++ b/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py @@ -56,6 +56,7 @@ def date_param(): "assets list", "assets create-event 1", "assets create-event --asset-id=1", + "assets get 1", # Backfill commands "backfill list", # Config commands From 2757621a2170c273f9e1fe3b6f937cd847dcd38b Mon Sep 17 00:00:00 2001 From: Fury0508 Date: Wed, 4 Feb 2026 13:10:23 +0530 Subject: [PATCH 09/16] Fix positional parameter implementation for both Pydantic and primitive types - Use original field names for positional args (not sanitized versions) - Apply positional logic to primitive type parameters (e.g., asset_id: str) - Update test fixtures to expect original field names for positional args - Required non-boolean parameters now appear as positional arguments - Boolean and optional parameters remain as flags with -- prefix This ensures commands like 'airflowctl assets get 1' work correctly --- airflow-ctl/src/airflowctl/ctl/cli_config.py | 6 +- .../tests/airflow_ctl/ctl/test_cli_config.py | 89 +++++++++---------- 2 files changed, 47 insertions(+), 48 deletions(-) diff --git a/airflow-ctl/src/airflowctl/ctl/cli_config.py b/airflow-ctl/src/airflowctl/ctl/cli_config.py index 4c4b277b04b6b..cbff66d111b20 100755 --- a/airflow-ctl/src/airflowctl/ctl/cli_config.py +++ b/airflow-ctl/src/airflowctl/ctl/cli_config.py @@ -552,7 +552,7 @@ def _create_arg_for_non_primitive_type( if type(field_type.annotation) is type: is_bool = field_type.annotation is bool - arg_flags = (sanitized_field,) if is_required and not is_bool else ("--" + sanitized_field,) + arg_flags = (field,) if is_required and not is_bool else ("--" + sanitized_field,) commands.append( self._create_arg( arg_flags=arg_flags, @@ -569,7 +569,7 @@ def _create_arg_for_non_primitive_type( annotation = field_type.annotation is_bool = annotation is bool - arg_flags = (sanitized_field,) if is_required and not is_bool else ("--" + sanitized_field,) + arg_flags = (field,) if is_required and not is_bool else ("--" + sanitized_field,) commands.append( self._create_arg( arg_flags=arg_flags, @@ -593,7 +593,7 @@ def _create_args_map_from_operation(self): sanitized_key = self._sanitize_arg_parameter_key(parameter_key) args.append( self._create_arg( - arg_flags=("--" + sanitized_key,), + arg_flags=(parameter_key,) if not is_bool else ("--" + sanitized_key,), arg_type=self._python_type_from_string(parameter_type), arg_action=argparse.BooleanOptionalAction if is_bool else None, arg_help=f"{parameter_key} for {operation.get('name')} operation in {operation.get('parent').name}", diff --git a/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py b/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py index 6b1b11eacd64d..374b13a4053cd 100644 --- a/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py +++ b/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py @@ -59,7 +59,7 @@ def no_op(): def test_args_create(): return [ ( - "dag-id", + "dag_id", { "help": "dag_id for backfill operation", "action": None, @@ -69,7 +69,7 @@ def test_args_create(): }, ), ( - "from-date", + "from_date", { "help": "from_date for backfill operation", "action": None, @@ -79,7 +79,7 @@ def test_args_create(): }, ), ( - "to-date", + "to_date", { "help": "to_date for backfill operation", "action": None, @@ -158,7 +158,7 @@ def test_args_list(): def test_args_get(): return [ ( - "--backfill-id", + "backfill_id", { "help": "backfill_id for get operation in BackfillsOperations", "default": None, @@ -182,7 +182,7 @@ def test_args_get(): def test_args_delete(): return [ ( - "--backfill-id", + "backfill_id", { "help": "backfill_id for delete operation in BackfillsOperations", "default": None, @@ -541,52 +541,52 @@ def test_add_auth_token_to_all_commands(self, no_op_method): assert command.args == expected_command_2_args def test_trigger_dag_run_defaults_logical_date_to_now(self): - """Test that trigger command defaults logical_date to now when not provided.""" - from datetime import datetime, timezone - - from airflowctl.api.datamodels.generated import TriggerDAGRunPostBody - - # Simulate the logic in _get_func from cli_config.py - # This is the actual code path that runs when user doesn't provide --logical-date - - # Step 1: Simulate CLI args being parsed (logical_date=None) - method_params = { - "trigger_dag_run": { - "dag_run_id": None, - "data_interval_start": None, - "data_interval_end": None, - "logical_date": None, # User did not provide --logical-date - "run_after": None, - "conf": None, - "note": None, - "partition_key": None, - } + """Test that trigger command defaults logical_date to now when not provided.""" + from datetime import datetime, timezone + + from airflowctl.api.datamodels.generated import TriggerDAGRunPostBody + + # Simulate the logic in _get_func from cli_config.py + # This is the actual code path that runs when user doesn't provide --logical-date + + # Step 1: Simulate CLI args being parsed (logical_date=None) + method_params = { + "trigger_dag_run": { + "dag_run_id": None, + "data_interval_start": None, + "data_interval_end": None, + "logical_date": None, # User did not provide --logical-date + "run_after": None, + "conf": None, + "note": None, + "partition_key": None, } + } - # Step 2: Apply the defaulting logic (from cli_config.py lines 622-630) - datamodel = TriggerDAGRunPostBody - datamodel_param_name = "trigger_dag_run" + # Step 2: Apply the defaulting logic (from cli_config.py lines 622-630) + datamodel = TriggerDAGRunPostBody + datamodel_param_name = "trigger_dag_run" - if ( - datamodel.__name__ == "TriggerDAGRunPostBody" - and "logical_date" in method_params[datamodel_param_name] - and method_params[datamodel_param_name]["logical_date"] is None - ): - method_params[datamodel_param_name]["logical_date"] = datetime.now(timezone.utc) + if ( + datamodel.__name__ == "TriggerDAGRunPostBody" + and "logical_date" in method_params[datamodel_param_name] + and method_params[datamodel_param_name]["logical_date"] is None + ): + method_params[datamodel_param_name]["logical_date"] = datetime.now(timezone.utc) - # Step 3: Create the Pydantic model (what happens in the actual code) - trigger_body = datamodel.model_validate(method_params[datamodel_param_name]) + # Step 3: Create the Pydantic model (what happens in the actual code) + trigger_body = datamodel.model_validate(method_params[datamodel_param_name]) - # Step 4: Verify logical_date was set to now - assert trigger_body.logical_date is not None, "logical_date should be defaulted to now" - assert isinstance(trigger_body.logical_date, datetime) + # Step 4: Verify logical_date was set to now + assert trigger_body.logical_date is not None, "logical_date should be defaulted to now" + assert isinstance(trigger_body.logical_date, datetime) - # Verify it's close to current time (within 5 seconds) - time_diff = abs((datetime.now(timezone.utc) - trigger_body.logical_date).total_seconds()) - assert time_diff < 5, f"logical_date should be close to now, but diff is {time_diff} seconds" + # Verify it's close to current time (within 5 seconds) + time_diff = abs((datetime.now(timezone.utc) - trigger_body.logical_date).total_seconds()) + assert time_diff < 5, f"logical_date should be close to now, but diff is {time_diff} seconds" - # Also verify timezone is UTC - assert trigger_body.logical_date.tzinfo is not None, "logical_date should have timezone info" + # Also verify timezone is UTC + assert trigger_body.logical_date.tzinfo is not None, "logical_date should have timezone info" def test_apply_datamodel_defaults_trigger_dag_run_with_none(self): """Test _apply_datamodel_defaults sets logical_date to now when None for TriggerDAGRunPostBody.""" @@ -689,4 +689,3 @@ def test_positional_args(self): arg_action=None, ) assert optional_arg.flags[0] == "--description" - From 89380574757323e03e43bfadefe437942babc099 Mon Sep 17 00:00:00 2001 From: Fury0508 Date: Wed, 4 Feb 2026 15:19:30 +0530 Subject: [PATCH 10/16] Update integration tests to use positional parameter syntax --- .../test_airflowctl_commands.py | 65 +++++++++---------- 1 file changed, 32 insertions(+), 33 deletions(-) diff --git a/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py b/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py index 5ad48f3bbc605..1c1117be14382 100644 --- a/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py +++ b/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py @@ -55,12 +55,11 @@ def date_param(): # Assets commands "assets list", "assets create-event 1", - "assets create-event --asset-id=1", "assets get 1", # Backfill commands - "backfill list", + "backfill list example_bash_operator", # Config commands - "config get --section core --option executor", + "config get core executor", "config list", "config lint", # Connections commands @@ -68,62 +67,62 @@ def date_param(): "connections list", "connections list -o yaml", "connections list -o table", - "connections get --conn-id=test_con", - "connections get --conn-id=test_con -o json", + "connections get test_con", + "connections get test_con -o json", "connections update test_con postgres", "connections import tests/airflowctl_tests/fixtures/test_connections.json", - "connections delete --conn-id=test_con", - "connections delete --conn-id=test_import_conn", + "connections delete test_con", + "connections delete test_import_conn", # DAGs commands "dags list", - "dags get --dag-id=example_bash_operator", - "dags get-details --dag-id=example_bash_operator", - "dags get-stats --dag-ids=example_bash_operator", - "dags get-version --dag-id=example_bash_operator --version-number=1", + "dags get example_bash_operator", + "dags get-details example_bash_operator", + "dags get-stats example_bash_operator", + "dags get-version example_bash_operator 1", "dags list-import-errors", - "dags list-version --dag-id=example_bash_operator", + "dags list-version example_bash_operator", "dags list-warning", # Order of trigger and pause/unpause is important for test stability because state checked - "dags trigger --dag-id=example_bash_operator --logical-date={date_param} --run-after={date_param}", + "dags trigger example_bash_operator --logical-date={date_param} --run-after={date_param}", # Test trigger without logical-date (should default to now) - "dags trigger --dag-id=example_bash_operator", + "dags trigger example_bash_operator", "dags pause example_bash_operator", "dags unpause example_bash_operator", # DAG Run commands - 'dagrun get --dag-id=example_bash_operator --dag-run-id="manual__{date_param}"', - "dags update --dag-id=example_bash_operator --no-is-paused", + 'dagrun get example_bash_operator "manual__{date_param}"', + "dags update example_bash_operator --no-is-paused", # DAG Run commands - "dagrun list --dag-id example_bash_operator --state success --limit=1", + "dagrun list example_bash_operator --state success --limit=1", # XCom commands - need a DAG run with completed tasks - 'xcom add --dag-id=example_bash_operator --dag-run-id="manual__{date_param}" --task-id=runme_0 --key={xcom_key} --value=\'{{"test": "value"}}\'', - 'xcom get --dag-id=example_bash_operator --dag-run-id="manual__{date_param}" --task-id=runme_0 --key={xcom_key}', - 'xcom list --dag-id=example_bash_operator --dag-run-id="manual__{date_param}" --task-id=runme_0', - 'xcom edit --dag-id=example_bash_operator --dag-run-id="manual__{date_param}" --task-id=runme_0 --key={xcom_key} --value=\'{{"updated": "value"}}\'', - 'xcom delete --dag-id=example_bash_operator --dag-run-id="manual__{date_param}" --task-id=runme_0 --key={xcom_key}', + 'xcom add example_bash_operator "manual__{date_param}" runme_0 {xcom_key} \'{{"test": "value"}}\'', + 'xcom get example_bash_operator "manual__{date_param}" runme_0 {xcom_key}', + 'xcom list example_bash_operator "manual__{date_param}" runme_0', + 'xcom edit example_bash_operator "manual__{date_param}" runme_0 {xcom_key} \'{{"updated": "value"}}\'', + 'xcom delete example_bash_operator "manual__{date_param}" runme_0 {xcom_key}', # Jobs commands "jobs list", # Pools commands "pools create test_pool 5", "pools list", - "pools get --pool-name=test_pool", - "pools get --pool-name=test_pool -o yaml", - "pools update --pool=test_pool --slots=10", + "pools get test_pool", + "pools get test_pool -o yaml", + "pools update test_pool --slots=10", "pools import tests/airflowctl_tests/fixtures/test_pools.json", "pools export tests/airflowctl_tests/fixtures/pools_export.json --output=json", - "pools delete --pool=test_pool", - "pools delete --pool=test_import_pool", + "pools delete test_pool", + "pools delete test_import_pool", # Providers commands "providers list", # Variables commands "variables create test_key test_value", "variables list", - "variables get --variable-key=test_key", - "variables get --variable-key=test_key -o table", + "variables get test_key", + "variables get test_key -o table", "variables update test_key updated_value", "variables import tests/airflowctl_tests/fixtures/test_variables.json", - "variables delete --variable-key=test_key", - "variables delete --variable-key=test_import_var", - "variables delete --variable-key=test_import_var_with_desc", + "variables delete test_key", + "variables delete test_import_var", + "variables delete test_import_var_with_desc", # Version command "version --remote", # Plugins command @@ -191,4 +190,4 @@ def test_airflowctl_commands_skip_keyring(command: str, api_token: str, run_comm "AIRFLOW_CLI_ENVIRONMENT": "nokeyring", }, skip_login=True, - ) + ) \ No newline at end of file From 7afc901d2fcc245edd4cbf254da09456b2741187 Mon Sep 17 00:00:00 2001 From: Fury0508 Date: Thu, 5 Feb 2026 10:05:19 +0530 Subject: [PATCH 11/16] Check for default values to determine positional vs optional parameters - Extract has_default from function signatures via AST - Only make parameters positional if: not boolean AND not has_default AND not in 'list' operation - Skip has_default metadata in both args_map and func_map creation - Fixes xcom operations (map_index has default) and list operations (filter params should be optional) --- airflow-ctl/src/airflowctl/ctl/cli_config.py | 22 ++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/airflow-ctl/src/airflowctl/ctl/cli_config.py b/airflow-ctl/src/airflowctl/ctl/cli_config.py index cbff66d111b20..762903d0b501a 100755 --- a/airflow-ctl/src/airflowctl/ctl/cli_config.py +++ b/airflow-ctl/src/airflowctl/ctl/cli_config.py @@ -423,11 +423,16 @@ def get_function_details(node: ast.FunctionDef, parent_node: ast.ClassDef) -> di args = [] return_annotation: str = "" - for arg in node.args.args: + num_args = len(node.args.args) + num_defaults = len(node.args.defaults) + first_default_index = num_args - num_defaults + + for idx, arg in enumerate(node.args.args): arg_name = arg.arg arg_type = ast.unparse(arg.annotation) if arg.annotation else "Any" if arg_name != "self": - args.append({arg_name: arg_type}) + has_default = idx >= first_default_index + args.append({arg_name: arg_type, "has_default": has_default}) if node.returns: return_annotation = [ @@ -586,14 +591,21 @@ def _create_args_map_from_operation(self): for operation in self.operations: args = [] for parameter in operation.get("parameters"): - for parameter_key, parameter_type in parameter.items(): + for parameter_key, parameter_value in parameter.items(): + if parameter_key == "has_default": + continue + parameter_type = parameter_value + has_default = parameter.get("has_default", False) + if self._is_primitive_type(type_name=parameter_type): base_parameter_type = parameter_type.replace(" | None", "").strip() is_bool = base_parameter_type == "bool" sanitized_key = self._sanitize_arg_parameter_key(parameter_key) + operation_name = operation.get("name") + is_positional = not is_bool and not has_default and operation_name != "list" args.append( self._create_arg( - arg_flags=(parameter_key,) if not is_bool else ("--" + sanitized_key,), + arg_flags=(parameter_key,) if is_positional else ("--" + sanitized_key,), arg_type=self._python_type_from_string(parameter_type), arg_action=argparse.BooleanOptionalAction if is_bool else None, arg_help=f"{parameter_key} for {operation.get('name')} operation in {operation.get('parent').name}", @@ -656,6 +668,8 @@ def _get_func(args: Namespace, api_operation: dict, api_client: Client = NEW_API args_dict = vars(args) for parameter in api_operation["parameters"]: for parameter_key, parameter_type in parameter.items(): + if parameter_key == "has_default": + continue if self._is_primitive_type(type_name=parameter_type): method_params[self._sanitize_method_param_key(parameter_key)] = args_dict[ parameter_key From ffbc53c0f09b6aa735b8729f65eb6904b79ccce8 Mon Sep 17 00:00:00 2001 From: Fury0508 Date: Thu, 5 Feb 2026 11:18:37 +0530 Subject: [PATCH 12/16] Implement positional parameters with default value detection - Extract has_default from AST to identify optional parameters - Make primitive parameters positional only if: not boolean, no default, and not in special list operations - Special handling for dagrun.list and jobs.list (all params optional despite no defaults) - Update integration tests to use correct syntax for optional parameters --- .../tests/airflowctl_tests/test_airflowctl_commands.py | 4 ++-- airflow-ctl/src/airflowctl/ctl/cli_config.py | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py b/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py index 1c1117be14382..f39e97fed33ac 100644 --- a/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py +++ b/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py @@ -92,7 +92,7 @@ def date_param(): 'dagrun get example_bash_operator "manual__{date_param}"', "dags update example_bash_operator --no-is-paused", # DAG Run commands - "dagrun list example_bash_operator --state success --limit=1", + "dagrun list --dag-id=example_bash_operator --state success --limit=1", # XCom commands - need a DAG run with completed tasks 'xcom add example_bash_operator "manual__{date_param}" runme_0 {xcom_key} \'{{"test": "value"}}\'', 'xcom get example_bash_operator "manual__{date_param}" runme_0 {xcom_key}', @@ -100,7 +100,7 @@ def date_param(): 'xcom edit example_bash_operator "manual__{date_param}" runme_0 {xcom_key} \'{{"updated": "value"}}\'', 'xcom delete example_bash_operator "manual__{date_param}" runme_0 {xcom_key}', # Jobs commands - "jobs list", + "jobs list --job-type=SchedulerJob --hostname=localhost", # Pools commands "pools create test_pool 5", "pools list", diff --git a/airflow-ctl/src/airflowctl/ctl/cli_config.py b/airflow-ctl/src/airflowctl/ctl/cli_config.py index 762903d0b501a..12105ad7e9a30 100755 --- a/airflow-ctl/src/airflowctl/ctl/cli_config.py +++ b/airflow-ctl/src/airflowctl/ctl/cli_config.py @@ -601,8 +601,10 @@ def _create_args_map_from_operation(self): base_parameter_type = parameter_type.replace(" | None", "").strip() is_bool = base_parameter_type == "bool" sanitized_key = self._sanitize_arg_parameter_key(parameter_key) - operation_name = operation.get("name") - is_positional = not is_bool and not has_default and operation_name != "list" + operation_name = operation.get('name') + parent_name = operation.get('parent').name + is_jobs_or_dagrun_list = operation_name == 'list' and parent_name in ['JobsOperations', 'DagRunOperations'] + is_positional = not is_bool and not has_default and not is_jobs_or_dagrun_list args.append( self._create_arg( arg_flags=(parameter_key,) if is_positional else ("--" + sanitized_key,), From 4db4242ced6cae0d227f0a499fe93274166e7789 Mon Sep 17 00:00:00 2001 From: Fury0508 Date: Thu, 5 Feb 2026 11:35:08 +0530 Subject: [PATCH 13/16] ran perk--all-files --- airflow-ctl/src/airflowctl/ctl/cli_config.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/airflow-ctl/src/airflowctl/ctl/cli_config.py b/airflow-ctl/src/airflowctl/ctl/cli_config.py index 12105ad7e9a30..444620b69c8b8 100755 --- a/airflow-ctl/src/airflowctl/ctl/cli_config.py +++ b/airflow-ctl/src/airflowctl/ctl/cli_config.py @@ -601,9 +601,12 @@ def _create_args_map_from_operation(self): base_parameter_type = parameter_type.replace(" | None", "").strip() is_bool = base_parameter_type == "bool" sanitized_key = self._sanitize_arg_parameter_key(parameter_key) - operation_name = operation.get('name') - parent_name = operation.get('parent').name - is_jobs_or_dagrun_list = operation_name == 'list' and parent_name in ['JobsOperations', 'DagRunOperations'] + operation_name = operation.get("name") + parent_name = operation.get("parent").name + is_jobs_or_dagrun_list = operation_name == "list" and parent_name in [ + "JobsOperations", + "DagRunOperations", + ] is_positional = not is_bool and not has_default and not is_jobs_or_dagrun_list args.append( self._create_arg( From 95b434c181de5d80f98c182e9a3fa796e0b14c43 Mon Sep 17 00:00:00 2001 From: Fury0508 Date: Thu, 5 Feb 2026 12:45:20 +0530 Subject: [PATCH 14/16] Implement positional parameters with default value detection - Extract has_default from AST to identify optional parameters - Make primitive parameters positional only if: not boolean, no default, and not in special list operations - Special handling for dagrun.list and jobs.list (all params optional despite no defaults) - Skip has_default metadata key in func_map creation - Update integration tests to use correct syntax: - dagrun list: use --dag-id flag - jobs list: add required --job-type and --hostname flags - pools update: use --pool flag (field is optional in Pydantic model) --- .../tests/airflowctl_tests/test_airflowctl_commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py b/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py index f39e97fed33ac..73904cd532ae7 100644 --- a/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py +++ b/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py @@ -106,7 +106,7 @@ def date_param(): "pools list", "pools get test_pool", "pools get test_pool -o yaml", - "pools update test_pool --slots=10", + "pools update --pool=test_pool --slots=10", "pools import tests/airflowctl_tests/fixtures/test_pools.json", "pools export tests/airflowctl_tests/fixtures/pools_export.json --output=json", "pools delete test_pool", From e515c2f6cef2f3bf59e5426b0774eb52d0ba805c Mon Sep 17 00:00:00 2001 From: Fury0508 Date: Wed, 22 Apr 2026 22:34:18 +0000 Subject: [PATCH 15/16] Fix positional args logic and update test fixtures --- airflow-ctl/src/airflowctl/ctl/cli_config.py | 13 ++++--------- .../tests/airflow_ctl/ctl/test_cli_config.py | 5 ++--- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/airflow-ctl/src/airflowctl/ctl/cli_config.py b/airflow-ctl/src/airflowctl/ctl/cli_config.py index 444620b69c8b8..3d6463643fd50 100755 --- a/airflow-ctl/src/airflowctl/ctl/cli_config.py +++ b/airflow-ctl/src/airflowctl/ctl/cli_config.py @@ -600,21 +600,16 @@ def _create_args_map_from_operation(self): if self._is_primitive_type(type_name=parameter_type): base_parameter_type = parameter_type.replace(" | None", "").strip() is_bool = base_parameter_type == "bool" + is_optional = "| None" in parameter_type sanitized_key = self._sanitize_arg_parameter_key(parameter_key) - operation_name = operation.get("name") - parent_name = operation.get("parent").name - is_jobs_or_dagrun_list = operation_name == "list" and parent_name in [ - "JobsOperations", - "DagRunOperations", - ] - is_positional = not is_bool and not has_default and not is_jobs_or_dagrun_list + arg_flags = ("--" + sanitized_key,) if is_optional or is_bool else (sanitized_key,) args.append( self._create_arg( - arg_flags=(parameter_key,) if is_positional else ("--" + sanitized_key,), + arg_flags=arg_flags, arg_type=self._python_type_from_string(parameter_type), arg_action=argparse.BooleanOptionalAction if is_bool else None, arg_help=f"{parameter_key} for {operation.get('name')} operation in {operation.get('parent').name}", - arg_default=False if is_bool else None, + arg_default=False if parameter_type == "bool" else None, ) ) else: diff --git a/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py b/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py index 374b13a4053cd..0c2937ea9c2f5 100644 --- a/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py +++ b/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py @@ -158,7 +158,7 @@ def test_args_list(): def test_args_get(): return [ ( - "backfill_id", + "backfill-id", { "help": "backfill_id for get operation in BackfillsOperations", "default": None, @@ -177,12 +177,11 @@ def test_args_get(): ), ] - @pytest.fixture(scope="module") def test_args_delete(): return [ ( - "backfill_id", + "backfill-id", { "help": "backfill_id for delete operation in BackfillsOperations", "default": None, From 790fad1c718e1defb1bd5a207c022424758de1da Mon Sep 17 00:00:00 2001 From: Fury0508 Date: Wed, 22 Apr 2026 22:42:08 +0000 Subject: [PATCH 16/16] Fix positional args logic and update test fixtures --- .../tests/airflowctl_tests/test_airflowctl_commands.py | 2 +- airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py b/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py index 73904cd532ae7..7bb609ca77c05 100644 --- a/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py +++ b/airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py @@ -190,4 +190,4 @@ def test_airflowctl_commands_skip_keyring(command: str, api_token: str, run_comm "AIRFLOW_CLI_ENVIRONMENT": "nokeyring", }, skip_login=True, - ) \ No newline at end of file + ) diff --git a/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py b/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py index 0c2937ea9c2f5..188d0565f17cd 100644 --- a/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py +++ b/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py @@ -177,11 +177,12 @@ def test_args_get(): ), ] + @pytest.fixture(scope="module") def test_args_delete(): return [ ( - "backfill-id", + "backfill-id", { "help": "backfill_id for delete operation in BackfillsOperations", "default": None, @@ -669,6 +670,7 @@ def test_help_texts_used_for_auto_generated_commands(self, group_name, subcomman "Help message should match the help_text.yaml" ) return + def test_positional_args(self): """Test that required parameters are created as positional arguments.""" command_factory = CommandFactory(file_path="")