Copilot commented on code in PR #67947:
URL: https://github.com/apache/airflow/pull/67947#discussion_r3572267182
##########
airflow-ctl/src/airflowctl/ctl/cli_config.py:
##########
@@ -52,6 +54,22 @@
BUILD_DOCS = "BUILDING_AIRFLOW_DOCS" in os.environ
+def _is_list_annotation(annotation: Any) -> bool:
+ """Check whether a Pydantic field annotation is a list type (including
Optional[list[...]])."""
+ origin = typing.get_origin(annotation)
+ if origin is list:
+ return True
+ # Handle both typing.Union (Optional[list[...]]) and PEP-604 X | Y
(types.UnionType)
+ if origin is typing.Union or isinstance(annotation,
builtin_types.UnionType):
+ return any(_is_list_annotation(arg) for arg in
typing.get_args(annotation) if arg is not type(None))
+ return False
+
+
+def _get_bool_flag_default(field_info: Any) -> bool:
+ """Return the CLI default for a generated bool flag: the datamodel field's
default (the API default), or False."""
+ return field_info.default if isinstance(field_info.default, bool) else
False
Review Comment:
_get_bool_flag_default() forces Optional[bool] fields with a datamodel
default of None (e.g. ClearTaskInstancesBody.run_on_latest_version) to default
to False. That changes API semantics: instead of omitting the field and letting
the server fall back to DAG-level/config defaults, the CLI will always send
false unless the user explicitly passes --run-on-latest-version, effectively
disabling the fallback chain.
##########
airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py:
##########
@@ -321,6 +321,44 @@ def list(self, is_alive: bool | None = None) ->
JobCollectionResponse | ServerRe
assert is_alive_arg.kwargs["default"] is None
assert is_alive_arg.kwargs["type"] is bool
+ def test_command_factory_body_bool_fields_use_datamodel_defaults(self,
tmp_path):
+ """Generated bool flags default to the datamodel field's default, so
the CLI mirrors the API defaults."""
+ temp_file = self._save_temp_operations_py(
+ tmp_path=tmp_path,
+ file_content="""
+ class TasksOperations(BaseOperations):
+ def clear(self, dag_id: str, body: ClearTaskInstancesBody):
+ self.response = self.client.post(
+ f"dags/{dag_id}/clearTaskInstances",
+ json=body.model_dump(mode="json"),
+ )
+ return self.response
+ """,
+ )
+
+ command_factory = CommandFactory(file_path=str(temp_file))
+ clear_args: list = []
+ for generated_group_command in command_factory.group_commands:
+ if generated_group_command.name != "tasks":
+ continue
+ for sub_command in generated_group_command.subcommands:
+ if sub_command.name == "clear":
+ clear_args = list(sub_command.args)
+ break
+
+ expected_defaults = {
+ "--dry-run": True,
+ "--only-failed": True,
+ "--reset-dag-runs": True,
+ "--include-upstream": False,
+ # datamodel default is None (fallback semantics), so the flag
falls back to False
+ "--run-on-latest-version": False,
+ }
Review Comment:
ClearTaskInstancesBody.run_on_latest_version has a default of None to enable
server-side fallback (DAG-level param → config → False). The test currently
asserts the CLI default is False, which would force an explicit false into the
request body and bypass that fallback behavior.
--
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]
