Shawnsuun commented on code in PR #74383:
URL: https://github.com/apache/airflow/pull/74383#discussion_r4431388930
##########
airflow-ctl/src/airflowctl/ctl/cli_config.py:
##########
@@ -779,11 +775,16 @@ def _find_subgroup_suffix(self, operation_group_name:
str, operation_name: str)
return suffix
return None
- 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 (
- field_default is None or isinstance(field_default, bool)
- ):
+ @staticmethod
+ def _get_bool_arg_default(field_default: Any) -> bool | None:
+ """
+ Get the default for a generated bool flag from its datamodel field
default.
+
+ An omitted flag must keep the API semantics: a bare ``tasks clear``
keeps ``dry_run=True``,
+ and an optional field such as ``is_paused`` or ``include_deferred``
stays ``None`` so it is
+ not sent at all instead of overwriting the stored value with ``False``.
+ """
+ if field_default is None or isinstance(field_default, bool):
return field_default
Review Comment:
Good catch. I added `backfill create` to the "User-visible change" section
of the description: an omitted `--run-on-latest-version` is no longer sent as
`False`, so the server now picks the value (the Dag's
`rerun_with_latest_version`, then `[core] rerun_with_latest_version`, then
`True` for backfills). A backfill that used to run on the original bundle
version now runs on the latest one by default, and `--no-run-on-latest-version`
keeps the old behavior.
##########
airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py:
##########
@@ -138,7 +138,7 @@ def date_param():
"pools list",
"pools get test_pool",
"pools get test_pool -o yaml",
- "pools update --pool=test_pool --slots=10",
+ "pools update --pool=test_pool --slots=10 --no-include-deferred",
Review Comment:
Added `related: #73768` and `related: #73931` to the description, with a
note that `pools update` without `--include-deferred` / `--no-include-deferred`
now gets a 422 until the server change in #73931 lets a PATCH send only the
changed fields.
##########
airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py:
##########
@@ -1427,6 +1438,105 @@ def
test_nested_single_key_list_is_not_split_into_rows(self, api_client_maker, c
assert [row["dag_run_id"] for row in rows] == ["manual_run"]
assert rows[0]["conf"] == ["a", "b"]
+ @pytest.mark.parametrize(
+ ("extra_argv", "expected_body"),
+ [
+ pytest.param(
+ ["--scheduling-state", "paused"],
+ DAGPatchBody(scheduling_state=DagSchedulingState.PAUSED),
+ id="scheduling-state-alone",
+ ),
+ pytest.param(["--is-paused"], DAGPatchBody(is_paused=True),
id="explicit-pause"),
+ pytest.param(["--no-is-paused"], DAGPatchBody(is_paused=False),
id="explicit-unpause"),
+ ],
+ )
+ @mock.patch.object(AirflowConsole, "print_as", autospec=True)
+ @mock.patch.object(DagsOperations, "update", autospec=True)
+ def test_dags_update_sends_only_given_state(
+ self, mocked_update, mocked_print_as, extra_argv, expected_body
+ ):
+ args = cli_parser.get_parser().parse_args(["dags", "update", "my_dag",
*extra_argv])
+
+ args.func(args, api_client=mock.MagicMock(spec=Client))
+
+ assert mocked_update.call_args.kwargs["dag_body"] == expected_body
+
+ @pytest.mark.parametrize(
+ "extra_argv",
+ [
+ pytest.param([], id="no-state-does-not-unpause"),
+ pytest.param(["--is-paused", "--scheduling-state", "paused"],
id="both-states"),
+ ],
+ )
+ @mock.patch.object(DagsOperations, "update", autospec=True)
Review Comment:
Done in ae1f9d2c98. `test_dags_update_requires_exactly_one_state` now
patches `AirflowConsole.print_as` like the test above. I checked it by
disabling the guard: it now fails with `DID NOT RAISE SystemExit` instead of
the tabulate `ValueError`.
--
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]
