This is an automated email from the ASF dual-hosted git repository.
shahar1 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 6f7cb34ea42 Keep airflowctl stdout parseable when a request is retried
or fails (#76347)
6f7cb34ea42 is described below
commit 6f7cb34ea42f85a9442d8351ed758dc6b711192d
Author: Andrew Chang <[email protected]>
AuthorDate: Tue Nov 3 23:53:30 2026 +0800
Keep airflowctl stdout parseable when a request is retried or fails (#76347)
The API client logs "Server error" and the tenacity retry warning
through structlog, which prints to stdout when nothing configures it.
A single retried 5xx then puts two log lines in front of the
`-o json` output, while the command still exits 0. The shared error
handler also printed its messages to stdout. #76133 and #76134 moved
the messages of a few commands to stderr, but these two paths are
shared by every command.
---
airflow-ctl/src/airflowctl/__main__.py | 5 ++
airflow-ctl/src/airflowctl/ctl/cli_config.py | 26 +++++----
.../tests/airflow_ctl/ctl/test_cli_config.py | 63 +++++++++++++++++++---
.../__main__.py => tests/airflow_ctl/test_main.py} | 35 +++++++-----
4 files changed, 97 insertions(+), 32 deletions(-)
diff --git a/airflow-ctl/src/airflowctl/__main__.py
b/airflow-ctl/src/airflowctl/__main__.py
index c8af217f250..781208b2839 100644
--- a/airflow-ctl/src/airflowctl/__main__.py
+++ b/airflow-ctl/src/airflowctl/__main__.py
@@ -21,13 +21,18 @@
from __future__ import annotations
+import sys
+
import argcomplete
+import structlog
from airflowctl.ctl import cli_parser
from airflowctl.ctl.cli_config import safe_call_command
def main():
+ # stdout carries only the command's output (e.g. ``-o json``); structlog
prints to stdout by default.
+
structlog.configure(logger_factory=structlog.PrintLoggerFactory(sys.stderr))
parser = cli_parser.get_parser()
argcomplete.autocomplete(parser)
args = parser.parse_args()
diff --git a/airflow-ctl/src/airflowctl/ctl/cli_config.py
b/airflow-ctl/src/airflowctl/ctl/cli_config.py
index c837a268f2a..a3f70533bc5 100755
--- a/airflow-ctl/src/airflowctl/ctl/cli_config.py
+++ b/airflow-ctl/src/airflowctl/ctl/cli_config.py
@@ -95,30 +95,35 @@ def safe_call_command(function: Callable, args:
Iterable[Arg]) -> None:
function(args)
except AirflowCtlException as e:
# The base class, so a new AirflowCtl error never regresses to a raw
traceback.
- rich.print(f"command failed due to {e}")
+ rich.print(f"command failed due to {e}", file=sys.stderr)
sys.exit(1)
except (httpx.RemoteProtocolError, httpx.ReadError, httpx.WriteError) as e:
- rich.print(f"[red]Remote protocol error: {e}[/red]")
+ rich.print(f"[red]Remote protocol error: {e}[/red]", file=sys.stderr)
if "Server disconnected without sending a response." in str(e):
rich.print(
f"[red]Server response error: {e}. "
- "Please check if the server is running and the API URL is
correct.[/red]"
+ "Please check if the server is running and the API URL is
correct.[/red]",
+ file=sys.stderr,
)
_warn_if_write_may_have_applied(e)
sys.exit(1)
except httpx.TimeoutException as e:
- rich.print(f"[red]Request timeout error: {e}[/red]")
+ rich.print(f"[red]Request timeout error: {e}[/red]", file=sys.stderr)
if "timed out" in str(e):
- rich.print("[red]Please check if the server is running and the API
ready to accept calls.[/red]")
+ rich.print(
+ "[red]Please check if the server is running and the API ready
to accept calls.[/red]",
+ file=sys.stderr,
+ )
_warn_if_write_may_have_applied(e)
sys.exit(1)
except ServerResponseError as e:
- rich.print(f"Server response error: {e}")
+ rich.print(f"Server response error: {e}", file=sys.stderr)
if "Client error message:" in str(e):
rich.print(
"[red]Client error, [/red] "
"Please check the command and its parameters. "
- "If you need help, run the command with --help."
+ "If you need help, run the command with --help.",
+ file=sys.stderr,
)
_warn_if_write_may_have_applied(e)
sys.exit(1)
@@ -126,11 +131,12 @@ def safe_call_command(function: Callable, args:
Iterable[Arg]) -> None:
# turn into a ``ServerResponseError`` -- a 3xx, or a 4xx/5xx whose body is
not JSON -- reach us
# as the bare httpx error.
except httpx.HTTPStatusError as e:
- rich.print(f"[red]Server response error: {e}[/red]")
+ rich.print(f"[red]Server response error: {e}[/red]", file=sys.stderr)
if e.response.is_redirect:
rich.print(
"[red]The server answered with a redirect, which airflowctl
does not follow. "
- "Please check that the API URL you logged in with points at
the Airflow API server.[/red]"
+ "Please check that the API URL you logged in with points at
the Airflow API server.[/red]",
+ file=sys.stderr,
)
_warn_if_write_may_have_applied(e)
sys.exit(1)
@@ -152,7 +158,7 @@ def safe_call_command(function: Callable, args:
Iterable[Arg]) -> None:
f"Response did not match the airflowctl model {detail}. "
"If the server is newer than this client, try upgrading
airflowctl."
)
- rich.print(f"[red]{message}[/red]")
+ rich.print(f"[red]{message}[/red]", file=sys.stderr)
sys.exit(1)
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 f0397cf65e8..1005ac29ad4 100644
--- a/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py
+++ b/airflow-ctl/tests/airflow_ctl/ctl/test_cli_config.py
@@ -794,6 +794,53 @@ class TestCliConfigMethods:
assert ctx.value.code == 1
+ @pytest.mark.parametrize(
+ ("raised_exception", "expected"),
+ [
+ pytest.param(
+ AirflowCtlNotFoundException("resource not found"),
+ ["command failed due to resource not found"],
+ id="airflowctl",
+ ),
+ pytest.param(
+ httpx.RemoteProtocolError("Server disconnected without sending
a response."),
+ ["Remote protocol error:", "check if the server is running and
the API URL is correct"],
+ id="protocol",
+ ),
+ pytest.param(
+ httpx.ReadTimeout("timed out"),
+ ["Request timeout error: timed out", "the API ready to accept
calls"],
+ id="timeout",
+ ),
+ pytest.param(
+ ServerResponseError.from_response(
+ httpx.Response(
+ 400,
+ request=httpx.Request("GET",
"http://localhost:8080/api/v2/dags"),
+ json={"detail": "bad"},
+ )
+ ),
+ [
+ "Server response error: Client error message:",
+ "Please check the command and its parameters",
+ ],
+ id="server-response",
+ ),
+ ],
+ )
+ def test_safe_call_command_reports_errors_on_stderr(self,
raised_exception, expected, capsys):
+ def raise_error(_args):
+ raise raised_exception
+
+ with pytest.raises(SystemExit):
+ safe_call_command(raise_error, args=argparse.Namespace())
+
+ captured = capsys.readouterr()
+ assert captured.out == ""
+ err = " ".join(captured.err.split())
+ for text in expected:
+ assert text in err
+
@pytest.mark.parametrize(
("response", "hint_expected"),
[
@@ -827,9 +874,9 @@ class TestCliConfigMethods:
assert ctx.value.code == 1
# Rich hard-wraps at the console width, so normalise before matching
on a phrase.
- out = " ".join(capsys.readouterr().out.split())
- assert "Server response error:" in out
- assert ("does not follow" in out) is hint_expected
+ err = " ".join(capsys.readouterr().err.split())
+ assert "Server response error:" in err
+ assert ("does not follow" in err) is hint_expected
@pytest.mark.parametrize(
("model", "payload", "expected", "unexpected"),
@@ -867,11 +914,11 @@ class TestCliConfigMethods:
safe_call_command(raise_validation_error,
args=argparse.Namespace())
assert ctx.value.code == 1
- out = " ".join(capsys.readouterr().out.split())
+ err = " ".join(capsys.readouterr().err.split())
for text in expected:
- assert text in out
+ assert text in err
for text in unexpected:
- assert text not in out
+ assert text not in err
def test_safe_call_command_reports_non_json_response(self, capsys):
def raise_validation_error(_args):
@@ -881,8 +928,8 @@ class TestCliConfigMethods:
safe_call_command(raise_validation_error,
args=argparse.Namespace())
assert ctx.value.code == 1
- out = " ".join(capsys.readouterr().out.split())
- assert "Server returned a non-JSON response for PoolResponse" in out
+ err = " ".join(capsys.readouterr().err.split())
+ assert "Server returned a non-JSON response for PoolResponse" in err
def test_add_to_parser_drops_type_for_boolean_optional_action(self):
"""Test add_to_parser removes type for BooleanOptionalAction."""
diff --git a/airflow-ctl/src/airflowctl/__main__.py
b/airflow-ctl/tests/airflow_ctl/test_main.py
similarity index 54%
copy from airflow-ctl/src/airflowctl/__main__.py
copy to airflow-ctl/tests/airflow_ctl/test_main.py
index c8af217f250..c483cd59261 100644
--- a/airflow-ctl/src/airflowctl/__main__.py
+++ b/airflow-ctl/tests/airflow_ctl/test_main.py
@@ -1,6 +1,3 @@
-#!/usr/bin/env python
-# PYTHON_ARGCOMPLETE_OK
-#
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
@@ -17,22 +14,32 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-"""Main executable module."""
from __future__ import annotations
-import argcomplete
+from unittest import mock
+
+import pytest
+import structlog
+
+from airflowctl import __main__
+from airflowctl.api import client
+
-from airflowctl.ctl import cli_parser
-from airflowctl.ctl.cli_config import safe_call_command
[email protected](autouse=True)
+def restore_structlog_config():
+ config = structlog.get_config()
+ yield
+ structlog.configure(**config)
-def main():
- parser = cli_parser.get_parser()
- argcomplete.autocomplete(parser)
- args = parser.parse_args()
- safe_call_command(args.func, args=args)
[email protected]("sys.argv", ["airflowctl", "version"])
[email protected](__main__, "safe_call_command", autospec=True)
+def test_main_writes_client_logs_to_stderr(mock_safe_call_command, capsys):
+ mock_safe_call_command.side_effect = lambda function, args:
client.log.warning("Server error ")
+ __main__.main()
-if __name__ == "__main__":
- main()
+ captured = capsys.readouterr()
+ assert captured.out == ""
+ assert "Server error" in captured.err