This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 9d17a46b40c Keep airflowctl operation defaults when an optional flag
is omitted (#65062)
9d17a46b40c is described below
commit 9d17a46b40ce403f23b87421cf7c27d91ac50a53
Author: rjgoyln <[email protected]>
AuthorDate: Thu Aug 13 20:45:54 2026 +0800
Keep airflowctl operation defaults when an optional flag is omitted (#65062)
* fix(cli): filter out None values to preserve method defaults
* fix CI test
* fix CI test
* fix: delete redifine func
* chore: apply pre-commit auto fixes
* Keep airflowctl operation defaults when an optional flag is omitted
argparse fills every generated optional flag with None, so a bare
``airflowctl dagrun list`` reached the operations method with
``limit=None`` and discarded the ``limit: int = 100`` that method
declares. Users silently got whatever the API server happens to default
to rather than the limit airflowctl documents.
The method signature is the single source of truth for those defaults,
so an omitted flag has to stay omitted rather than be forwarded as None.
Parameters that already default to None are unaffected — an explicit
filter must stay explicit.
---
airflow-ctl/src/airflowctl/ctl/cli_config.py | 17 ++++++--
.../tests/airflow_ctl/ctl/test_cli_config.py | 51 +++++++++++++++++++++-
2 files changed, 64 insertions(+), 4 deletions(-)
diff --git a/airflow-ctl/src/airflowctl/ctl/cli_config.py
b/airflow-ctl/src/airflowctl/ctl/cli_config.py
index bbf5e59f8e9..8a2bfd2e15b 100755
--- a/airflow-ctl/src/airflowctl/ctl/cli_config.py
+++ b/airflow-ctl/src/airflowctl/ctl/cli_config.py
@@ -552,6 +552,14 @@ class CommandFactory:
defaults_count = len(node.args.defaults)
required_count = len(positional_args) - defaults_count
required_param_names: set[str] = {a.arg for a in
positional_args[:required_count]}
+ # Parameters whose signature default carries real meaning (e.g.
``limit: int = 100``).
+ # ``argparse`` fills an omitted flag with ``None``, so these have
to be dropped from
+ # the call instead of being forwarded, or the method never sees
its own default.
+ non_none_default_param_names: set[str] = {
+ arg.arg
+ for arg, default in zip(positional_args[required_count:],
node.args.defaults)
+ if not (isinstance(default, ast.Constant) and default.value is
None)
+ }
for arg in positional_args:
arg_name = arg.arg
@@ -567,6 +575,7 @@ class CommandFactory:
"name": func_name,
"parameters": args,
"required_param_names": required_param_names,
+ "non_none_default_param_names": non_none_default_param_names,
"return_type": return_annotation,
"parent": parent_node,
}
@@ -837,12 +846,14 @@ class CommandFactory:
datamodel = None
datamodel_param_name = None
args_dict = vars(args)
+ non_none_default_param_names =
api_operation.get("non_none_default_param_names") or set()
for parameter in api_operation["parameters"]:
for parameter_key, parameter_type in parameter.items():
if self._is_primitive_type(type_name=parameter_type):
-
method_params[self._sanitize_method_param_key(parameter_key)] = args_dict[
- parameter_key
- ]
+ value = args_dict[parameter_key]
+ if value is None and parameter_key in
non_none_default_param_names:
+ continue
+
method_params[self._sanitize_method_param_key(parameter_key)] = value
else:
datamodel = getattr(generated_datamodels,
parameter_type)
for expanded_parameter in
self.datamodels_extended_map[parameter_type]:
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 fcbd3d748b2..5973a7c54d9 100644
--- a/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py
+++ b/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py
@@ -21,12 +21,13 @@ import argparse
from argparse import BooleanOptionalAction
from pathlib import Path
from textwrap import dedent
+from unittest import mock
import httpx
import pytest
from airflowctl.api.datamodels.generated import ClearTaskInstancesBody
-from airflowctl.api.operations import ServerResponseError
+from airflowctl.api.operations import DagRunOperations, ServerResponseError
from airflowctl.ctl.cli_config import (
ARG_AUTH_TOKEN,
ActionCommand,
@@ -850,3 +851,51 @@ class TestCliConfigMethods:
)
return
pytest.fail(f"Auto-generated command not found: {group_name}
{subcommand_name}")
+
+ @staticmethod
+ def _call_generated_command(monkeypatch, operations_class, method_name:
str, **parsed_args):
+ """Run the auto-generated command for ``operations_class.method_name``
and return its call kwargs."""
+
monkeypatch.setattr("airflowctl.ctl.cli_config.AirflowConsole.print_as", lambda
*_, **__: None)
+
+ command_factory = CommandFactory()
+ command_factory._inspect_operations()
+ operation = next(
+ op
+ for op in command_factory.operations
+ if op["name"] == method_name and op["parent"].name ==
operations_class.__name__
+ )
+ command_factory.operations = [operation]
+ command_factory._create_func_map_from_operation()
+
+ namespace = argparse.Namespace(
+ output="json",
+ **{key: parsed_args.get(key) for parameter in
operation["parameters"] for key in parameter},
+ )
+ with mock.patch.object(operations_class, method_name, autospec=True)
as mocked_method:
+ command_factory.func_map[(method_name, operations_class.__name__)](
+ namespace, api_client=mock.MagicMock()
+ )
+ return mocked_method.call_args.kwargs
+
+ @pytest.mark.parametrize(
+ ("parsed_limit", "limit_is_forwarded"),
+ [
+ pytest.param(None, False,
id="omitted-flag-keeps-signature-default"),
+ pytest.param(25, True,
id="explicit-flag-overrides-signature-default"),
+ ],
+ )
+ def test_primitive_param_with_non_none_default_is_not_clobbered(
+ self, monkeypatch, parsed_limit, limit_is_forwarded
+ ):
+ """``DagRunOperations.list`` declares ``limit: int = 100``; argparse's
None must not override it."""
+ call_kwargs = self._call_generated_command(monkeypatch,
DagRunOperations, "list", limit=parsed_limit)
+
+ assert ("limit" in call_kwargs) is limit_is_forwarded
+ if limit_is_forwarded:
+ assert call_kwargs["limit"] == parsed_limit
+
+ def test_primitive_param_defaulting_to_none_is_still_forwarded(self,
monkeypatch):
+ """Only a non-None signature default is worth protecting, so ``state:
str | None = None`` still goes through."""
+ call_kwargs = self._call_generated_command(monkeypatch,
DagRunOperations, "list")
+
+ assert call_kwargs["state"] is None
