This is an automated email from the ASF dual-hosted git repository.
henry3260 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 eaff9ebc6b0 Fix airflowctl commands crashing instead of printing their
result (#72675) (#73988)
eaff9ebc6b0 is described below
commit eaff9ebc6b033f8c9d3f7f76feed7e4dc6254f49
Author: Henry Chen <[email protected]>
AuthorDate: Thu Oct 1 11:33:37 2026 +0800
Fix airflowctl commands crashing instead of printing their result (#72675)
(#73988)
airflowctl generates its commands from the operations layer, and every
generated command finishes by printing through ``args.output``. That flag
was only declared for commands whose method name happened to start with
one of ten CRUD verbs, so the ones that did not — ``assets materialize``,
``backfill pause|unpause|cancel`` and ``connections test`` — reached the
printer with the attribute undefined and died with a raw traceback, after
their request had already been sent and applied server-side.
``-e/--env`` deliberately keeps its existing whitelist. Nothing reads
``args.env`` for generated commands, so widening it would only let more
commands silently accept an environment they then ignore and run against
production credentials instead; that is tracked separately in
https://github.com/apache/airflow/issues/70519.
Co-authored-by: Eason09053360
<[email protected]>
(cherry picked from commit f31b572df18639acf75781aeae8628732e6e2eab)
The pick is not clean, because this branch does not carry the commit that
introduced ``_call_generated_command`` and the ``test_primitive_param_*``
tests. Git offered those alongside this change in the conflict; they are
left out, since they belong to that commit rather than this one, as do the
``ClearTaskInstancesBody`` and ``DagRunOperations`` imports they need.
``from unittest import mock`` is plain context on main and is added here
for the same reason: the test this commit adds needs it, and the commit
that would have brought it is not on this branch.
Co-authored-by: Y-C <[email protected]>
---
airflow-ctl/src/airflowctl/ctl/cli_config.py | 12 ++++----
.../tests/airflow_ctl/ctl/test_cli_config.py | 34 +++++++++++++++++++++-
2 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/airflow-ctl/src/airflowctl/ctl/cli_config.py
b/airflow-ctl/src/airflowctl/ctl/cli_config.py
index e19a3fd62d6..4484b2e270a 100755
--- a/airflow-ctl/src/airflowctl/ctl/cli_config.py
+++ b/airflow-ctl/src/airflowctl/ctl/cli_config.py
@@ -408,7 +408,7 @@ class CommandFactory:
func_map: dict[tuple, Callable]
commands_map: dict[str, list[ActionCommand]]
group_commands_list: list[CLICommand]
- output_command_list: list[str]
+ auth_environment_command_list: list[str]
exclude_operation_names: list[str]
exclude_method_names: list[str]
help_texts: dict[str, dict[str, str]]
@@ -425,8 +425,7 @@ class CommandFactory:
# Excluded Lists are in Class Level for further usage and avoid
searching them
# Exclude parameters that are not needed for CLI from datamodels
self.excluded_parameters = ["schema_"]
- # This list is used to determine if the command/operation needs to
output data
- self.output_command_list = [
+ self.auth_environment_command_list = [
"list",
"get",
"create",
@@ -694,8 +693,11 @@ class CommandFactory:
)
)
- if any(operation.get("name").startswith(cmd) for cmd in
self.output_command_list):
- args.extend([ARG_OUTPUT, ARG_AUTH_ENVIRONMENT])
+ args.append(ARG_OUTPUT)
+ # ``-e/--env`` is inert here (nothing reads ``args.env``), so
widening it would only let more
+ # commands accept it and silently target production:
https://github.com/apache/airflow/issues/70519
+ if any(operation.get("name").startswith(cmd) for cmd in
self.auth_environment_command_list):
+ args.append(ARG_AUTH_ENVIRONMENT)
self.args_map[(operation.get("name"),
operation.get("parent").name)] = args
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 8d358c47830..2dfe81bba8d 100644
--- a/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py
+++ b/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py
@@ -21,13 +21,18 @@ 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.operations import ServerResponseError
+from airflowctl.api.client import Client
+from airflowctl.api.datamodels.generated import ConnectionTestResponse
+from airflowctl.api.operations import ConnectionsOperations,
ServerResponseError
+from airflowctl.ctl import cli_parser
from airflowctl.ctl.cli_config import (
ARG_AUTH_TOKEN,
+ ARG_OUTPUT,
ActionCommand,
Arg,
CommandFactory,
@@ -37,6 +42,7 @@ from airflowctl.ctl.cli_config import (
merge_commands,
safe_call_command,
)
+from airflowctl.ctl.console_formatting import AirflowConsole
from airflowctl.exceptions import (
AirflowCtlConnectionException,
AirflowCtlCredentialNotFoundException,
@@ -454,6 +460,19 @@ class TestCommandFactory:
assert limit_arg.flags == ("--limit",)
assert limit_arg.kwargs["type"] is int
+ 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()
+
+ missing = [
+ f"{group_command.name} {sub_command.name}"
+ for group_command in command_factory.group_commands
+ for sub_command in group_command.subcommands
+ if ARG_OUTPUT not in sub_command.args
+ ]
+
+ assert missing == []
+
class TestCliConfigMethods:
@pytest.mark.parametrize(
@@ -834,3 +853,16 @@ class TestCliConfigMethods:
"Help message should match the help_text.yaml"
)
return
+
+ @mock.patch.object(AirflowConsole, "print_as", autospec=True)
+ @mock.patch.object(ConnectionsOperations, "test", autospec=True)
+ def test_connections_test_reaches_the_printer(self, mocked_test,
mocked_print_as):
+ """``connections test`` has no CRUD-verb prefix, so argparse used to
leave ``args.output`` undefined."""
+ mocked_test.return_value = ConnectionTestResponse(status=True,
message="ok")
+ args = cli_parser.get_parser().parse_args(
+ ["connections", "test", "--connection-id", "my_conn",
"--conn-type", "http"]
+ )
+
+ args.func(args, api_client=mock.MagicMock(spec=Client))
+
+ assert mocked_print_as.call_args.kwargs["output"] == "json"
