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

Reply via email to