This is an automated email from the ASF dual-hosted git repository.
shahar1 pushed a commit to branch airflow-ctl/v0-1-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/airflow-ctl/v0-1-test by this
push:
new aba212112bd [airflow-ctl/v0-1-test] Keep airflowctl operation defaults
when an optional flag is omitted (#65062) (#76046)
aba212112bd is described below
commit aba212112bd947a363b740a1b2a084992de1ec6c
Author: Shahar Epstein <[email protected]>
AuthorDate: Fri Oct 30 17:56:26 2026 +0200
[airflow-ctl/v0-1-test] Keep airflowctl operation defaults when an optional
flag is omitted (#65062) (#76046)
* 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.
(cherry picked from commit 9d17a46b40ce403f23b87421cf7c27d91ac50a53)
Co-authored-by: rjgoyln <[email protected]>
---
airflow-ctl/src/airflowctl/ctl/cli_config.py | 17 ++++++--
.../tests/airflow_ctl/ctl/test_cli_config.py | 50 +++++++++++++++++++++-
2 files changed, 63 insertions(+), 4 deletions(-)
diff --git a/airflow-ctl/src/airflowctl/ctl/cli_config.py
b/airflow-ctl/src/airflowctl/ctl/cli_config.py
index 7719e969cfd..389afbc66cb 100755
--- a/airflow-ctl/src/airflowctl/ctl/cli_config.py
+++ b/airflow-ctl/src/airflowctl/ctl/cli_config.py
@@ -510,6 +510,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
@@ -525,6 +533,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,
}
@@ -795,12 +804,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 999bd9305cf..955c554b58e 100644
--- a/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py
+++ b/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py
@@ -31,7 +31,7 @@ import pytest
from airflowctl.api.client import Client
from airflowctl.api.datamodels.generated import ClearTaskInstancesBody,
ConnectionTestResponse
-from airflowctl.api.operations import ConnectionsOperations,
ServerResponseError
+from airflowctl.api.operations import ConnectionsOperations, DagRunOperations,
ServerResponseError
from airflowctl.ctl import cli_parser
from airflowctl.ctl.cli_config import (
ARG_AUTH_TOKEN,
@@ -1099,3 +1099,51 @@ class TestCliConfigMethods:
rows = json.loads(capsys.readouterr().out)
assert [row["dag_run_id"] for row in rows] == ["manual_run"]
assert rows[0]["conf"] == ["a", "b"]
+
+ @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
