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 0a118a8c645 [airflow-ctl/v0-1-test] Fix airflowctl operations create
command validation error (#69317) (#76116)
0a118a8c645 is described below
commit 0a118a8c645075da559a5570bcf0755d17c840dd
Author: Shahar Epstein <[email protected]>
AuthorDate: Sat Oct 31 09:39:24 2026 +0200
[airflow-ctl/v0-1-test] Fix airflowctl operations create command validation
error (#69317) (#76116)
* [airflow-ctl/v0-1-test] Fix airflowctl operations create command
validation error (#69317)
The only conflict was context: _find_subgroup_suffix from the operation
subgroups change is not on this branch and stays out. The fix itself applies
unchanged.
(cherry picked from commit 1c3b3def8694cbd96bad9f82af80d532386e1f04)
Co-authored-by: Pratiksha Badheka <[email protected]>
* Fix airflowctl required-option test broken by concurrent merges (#76092)
#69317 made required request-body fields required in argparse, and
#75177 added a test expecting the post-parse validation to report a
missing --conn-type. Each passed on its own, but together argparse
rejects the command during parsing, so the test failed on main. The
argparse error is the intended behaviour; the test now checks it.
(cherry picked from commit 9c164b5891f66a96a6bf886c8fa815e12c21394c)
---------
Co-authored-by: Pratiksha Badheka <[email protected]>
---
airflow-ctl/src/airflowctl/ctl/cli_config.py | 21 ++++++-
.../tests/airflow_ctl/api/test_operations.py | 8 ++-
.../tests/airflow_ctl/ctl/test_cli_config.py | 66 ++++++++++++++++++----
3 files changed, 80 insertions(+), 15 deletions(-)
diff --git a/airflow-ctl/src/airflowctl/ctl/cli_config.py
b/airflow-ctl/src/airflowctl/ctl/cli_config.py
index d6325678f52..c967572002f 100755
--- a/airflow-ctl/src/airflowctl/ctl/cli_config.py
+++ b/airflow-ctl/src/airflowctl/ctl/cli_config.py
@@ -27,6 +27,7 @@ import inspect
import json
import os
import sys
+import typing
from argparse import Namespace
from collections.abc import Callable, Iterable
from enum import Enum
@@ -641,6 +642,7 @@ class CommandFactory:
arg_action: argparse.BooleanOptionalAction | None,
arg_dest: str | None = None,
arg_default: Any | None = None,
+ arg_required: bool = False,
) -> Arg:
return Arg(
flags=arg_flags,
@@ -649,6 +651,7 @@ class CommandFactory:
help=arg_help,
default=arg_default,
action=arg_action,
+ required=True if arg_required else _UNSET,
)
@staticmethod
@@ -671,6 +674,11 @@ class CommandFactory:
help=arg_help,
)
+ @staticmethod
+ def _allows_none(annotation: Any) -> bool:
+ """Return True if the annotation accepts None, e.g. ``str | None``."""
+ return type(None) in typing.get_args(annotation)
+
def _get_bool_arg_default(self, parameter_type: str, field_default: Any)
-> bool | None:
"""Get default for a generated bool flag: the datamodel field default
for datamodels in ``field_bool_default_datamodels``, otherwise False."""
if parameter_type in self.field_bool_default_datamodels and (
@@ -707,9 +715,16 @@ class CommandFactory:
arg_type=self._python_type_from_string(annotation),
arg_action=argparse.BooleanOptionalAction if annotation is
bool else None, # type: ignore
arg_help=f"{field} for {parameter_key} operation",
- arg_default=self._get_bool_arg_default(parameter_type,
field_type.default)
- if annotation is bool
- else None,
+ arg_default=(
+ self._get_bool_arg_default(parameter_type,
field_type.default)
+ if annotation is bool
+ else None
+ ),
+ arg_required=(
+ field_type.is_required() # no default?
+ and annotation is not bool # not a bool flag
+ and not self._allows_none(field_type.annotation) #
None not acceptable
+ ),
)
)
return commands
diff --git a/airflow-ctl/tests/airflow_ctl/api/test_operations.py
b/airflow-ctl/tests/airflow_ctl/api/test_operations.py
index fb883b2e229..31dc3219ff9 100644
--- a/airflow-ctl/tests/airflow_ctl/api/test_operations.py
+++ b/airflow-ctl/tests/airflow_ctl/api/test_operations.py
@@ -900,7 +900,9 @@ class TestConnectionsOperations:
return httpx.Response(200,
json=json.loads(self.connection_response.model_dump_json()))
client = make_api_client(transport=httpx.MockTransport(handle_request))
- response = client.connections.create(connection=self.connection)
+ response = client.connections.create(
+ connection=self.connection,
+ )
assert response == self.connection_response
def test_create_uses_schema_alias_in_request_body(self):
@@ -922,7 +924,9 @@ class TestConnectionsOperations:
return httpx.Response(200,
json=json.loads(self.connection_response.model_dump_json()))
client = make_api_client(transport=httpx.MockTransport(handle_request))
- response = client.connections.create(connection=connection)
+ response = client.connections.create(
+ connection=connection,
+ )
assert response == self.connection_response
def test_bulk(self):
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 5addd3d2a55..262cc9c7d32 100644
--- a/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py
+++ b/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py
@@ -536,6 +536,47 @@ class TestCommandFactory:
assert limit_arg.flags == ("--limit",)
assert limit_arg.kwargs["type"] is int
+ @pytest.mark.parametrize(
+ ("group", "command", "required_flags"),
+ [
+ pytest.param(
+ "connections", "create", {"--connection-id", "--conn-type"},
id="connections-create"
+ ),
+ pytest.param("pools", "create", {"--name", "--slots"},
id="pools-create"),
+ ],
+ )
+ def test_required_datamodel_fields_become_required_flags(self, group,
command, required_flags):
+ """Required fields of a body model are generated as required
``--flags``."""
+ command_factory = CommandFactory()
+ group_command = next(g for g in command_factory.group_commands if
g.name == group)
+ sub_command = next(c for c in group_command.subcommands if c.name ==
command)
+
+ required = {arg.flags[0] for arg in sub_command.args if
arg.kwargs.get("required") is True}
+ assert required == required_flags
+
+ for arg in sub_command.args:
+ action = arg.kwargs.get("action")
+ if action is BooleanOptionalAction:
+ assert not arg.kwargs.get("required"), "bool flags always have
a default"
+
+ def test_optional_datamodel_fields_default_to_none(self):
+ """Optional non-bool fields stay ``None`` so Pydantic sees them as not
provided."""
+ command_factory = CommandFactory()
+ group_command = next(g for g in command_factory.group_commands if
g.name == "connections")
+ sub_command = next(c for c in group_command.subcommands if c.name ==
"create")
+
+ parser = argparse.ArgumentParser()
+ for arg in sub_command.args:
+ arg.add_to_parser(parser)
+
+ namespace = parser.parse_args(["--connection-id", "c1", "--conn-type",
"http"])
+
+ assert namespace.connection_id == "c1"
+ assert namespace.conn_type == "http"
+ assert namespace.host is None
+ assert namespace.login is None
+ assert namespace.description is None
+
def test_every_generated_command_accepts_the_output_flag(self):
"""``_get_func`` always prints through ``args.output``, so every
generated command must declare it."""
command_factory = CommandFactory()
@@ -549,6 +590,15 @@ class TestCommandFactory:
assert missing == []
+ def test_required_but_nullable_datamodel_field_stays_optional(self):
+ """A field with no default that accepts None (e.g. ``logical_date``)
must not become a required flag."""
+ command_factory = CommandFactory()
+ dags_group = next(g for g in command_factory.group_commands if g.name
== "dags")
+ trigger = next(c for c in dags_group.subcommands if c.name ==
"trigger")
+
+ logical_date_arg = next(arg for arg in trigger.args if arg.flags ==
("--logical-date",))
+ assert not logical_date_arg.kwargs.get("required")
+
class TestCliConfigMethods:
@pytest.mark.parametrize("method", ["GET", "HEAD", "OPTIONS", "POST",
"PUT", "PATCH", "DELETE"])
@@ -1050,19 +1100,15 @@ class TestCliConfigMethods:
assert "--port" in message
assert "required" not in message
- def
test_create_command_without_required_option_exits_with_friendly_message(self):
- """Test ``connections create`` without --conn-type exits cleanly and
never calls the API."""
+ def
test_create_command_without_required_option_is_rejected_by_parser(self, capsys):
+ """Test argparse rejects ``connections create`` without --conn-type:
exit code 2, message on stderr."""
parser = cli_parser.get_parser()
- args = parser.parse_args(
- ["connections", "create", "--api-token", "token",
"--connection-id", "my_conn"]
- )
- with mock.patch.object(ConnectionsOperations, "create") as mock_create:
- with pytest.raises(SystemExit) as ctx:
- args.func(args)
+ with pytest.raises(SystemExit) as ctx:
+ parser.parse_args(["connections", "create", "--api-token",
"token", "--connection-id", "my_conn"])
- assert "the following arguments are required: --conn-type" in
str(ctx.value)
- mock_create.assert_not_called()
+ assert ctx.value.code == 2
+ assert "the following arguments are required: --conn-type" in
capsys.readouterr().err
@pytest.mark.parametrize(
("group_name", "subcommand_name", "expected_help"),
