This is an automated email from the ASF dual-hosted git repository.
potiuk 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 079818a48a2 [airflow-ctl/v0-1-test] Fix a bad retry env var taking
down the whole airflowctl CLI (#72429) (#72663)
079818a48a2 is described below
commit 079818a48a26cf15789a9639fc717ddef2daf634
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Mon Sep 7 20:40:27 2026 +0200
[airflow-ctl/v0-1-test] Fix a bad retry env var taking down the whole
airflowctl CLI (#72429) (#72663)
The three retry knobs are coerced with int() at module scope, and every
airflowctl invocation imports that module, so a value the operator never
meant to set - an empty string from `docker run -e VAR`, an unexpanded CI
template variable, a typo - left them unable to run even `--help` to find
out what they had broken. A negative wait was worse still: it parsed
cleanly and then surfaced intermittently as a sleep-length error from deep
inside the retry layer, nowhere near its cause.
(cherry picked from commit 1be286ecfe49f3be3ab842dff5cadba497d7ceaf)
Co-authored-by: Y-C <[email protected]>
Co-authored-by: Eason09053360
<[email protected]>
---
airflow-ctl/docs/cli-and-env-variables-ref.rst | 7 ++++
airflow-ctl/src/airflowctl/api/client.py | 28 ++++++++++++--
airflow-ctl/tests/airflow_ctl/api/test_client.py | 48 ++++++++++++++++++++++++
3 files changed, 80 insertions(+), 3 deletions(-)
diff --git a/airflow-ctl/docs/cli-and-env-variables-ref.rst
b/airflow-ctl/docs/cli-and-env-variables-ref.rst
index 66e3638a4a7..dd60bfd5323 100644
--- a/airflow-ctl/docs/cli-and-env-variables-ref.rst
+++ b/airflow-ctl/docs/cli-and-env-variables-ref.rst
@@ -78,3 +78,10 @@ Environment Variables
The maximum amount of time to wait between API retries in seconds.
This is only used if you are using the Airflow API and have not set up
authentication using a different method. The default value is 10 seconds.
+
+.. note::
+
+ A value for any of the three retry variables above that is not a whole
number, or that is
+ out of range (below 1 for :envvar:`AIRFLOW_CLI_API_RETRIES`, negative for
the two wait
+ variables), is reported on standard error and the default is used instead.
Setting a
+ variable to an empty string is treated the same as leaving it unset.
diff --git a/airflow-ctl/src/airflowctl/api/client.py
b/airflow-ctl/src/airflowctl/api/client.py
index 1cdcf0ffca2..11fc0b487c9 100644
--- a/airflow-ctl/src/airflowctl/api/client.py
+++ b/airflow-ctl/src/airflowctl/api/client.py
@@ -323,10 +323,32 @@ def _should_retry_api_request(exception: BaseException)
-> bool:
return isinstance(exception, httpx.RequestError)
+def _get_int_env(name: str, default: int, minimum: int = 0) -> int:
+ """Read an integer tuning knob, keeping a bad value from taking the whole
CLI down at import."""
+ value = os.getenv(name)
+ # Docker and Kubernetes spell "not configured" as an empty value, not an
absent one.
+ if not value:
+ return default
+
+ parsed: int | None
+ try:
+ parsed = int(value)
+ except ValueError:
+ parsed = None
+ if parsed is not None and parsed >= minimum:
+ return parsed
+
+ # stderr, not the logger: logging is unconfigured at import and structlog
would use stdout.
+ sys.stderr.write(
+ f"Warning: ignoring {name}={value!r}, expected an integer >=
{minimum}; using {default}.\n"
+ )
+ return default
+
+
# API Client Retry Configuration
-API_RETRIES = int(os.getenv("AIRFLOW_CLI_API_RETRIES", "3"))
-API_RETRY_WAIT_MIN = int(os.getenv("AIRFLOW_CLI_API_RETRY_WAIT_MIN", "1"))
-API_RETRY_WAIT_MAX = int(os.getenv("AIRFLOW_CLI_API_RETRY_WAIT_MAX", "10"))
+API_RETRIES = _get_int_env("AIRFLOW_CLI_API_RETRIES", 3, minimum=1)
+API_RETRY_WAIT_MIN = _get_int_env("AIRFLOW_CLI_API_RETRY_WAIT_MIN", 1)
+API_RETRY_WAIT_MAX = _get_int_env("AIRFLOW_CLI_API_RETRY_WAIT_MAX", 10)
class Client(httpx.Client):
diff --git a/airflow-ctl/tests/airflow_ctl/api/test_client.py
b/airflow-ctl/tests/airflow_ctl/api/test_client.py
index dc33092e780..b7e50073851 100644
--- a/airflow-ctl/tests/airflow_ctl/api/test_client.py
+++ b/airflow-ctl/tests/airflow_ctl/api/test_client.py
@@ -20,6 +20,8 @@ from __future__ import annotations
import json
import os
import shutil
+import subprocess
+import sys
import tempfile
from unittest.mock import MagicMock, patch
@@ -523,3 +525,49 @@ def
test_credentials_rejects_unsafe_env_from_environment_variable(monkeypatch, a
monkeypatch.setenv("AIRFLOW_CLI_ENVIRONMENT", api_environment)
with pytest.raises(AirflowCtlException, match="environment"):
Credentials(client_kind=ClientKind.CLI)
+
+
+class TestRetryConfigurationEnvVars:
+ """The knobs are read at import time, so a bad value used to take down
even ``--help``."""
+
+ @staticmethod
+ def _import_with(**env: str) -> subprocess.CompletedProcess:
+ return subprocess.run(
+ [sys.executable, "-c", "import airflowctl.api.client as c;
print(c.API_RETRIES)"],
+ env={**os.environ, **env},
+ check=False,
+ text=True,
+ capture_output=True,
+ )
+
+ @pytest.mark.parametrize(
+ ("value", "expected", "warns"),
+ [
+ pytest.param("7", "7", False, id="integer-is-used"),
+ pytest.param("", "3", False, id="empty-means-unset"),
+ pytest.param("abc", "3", True, id="typo-falls-back"),
+ pytest.param("-5", "3", True, id="negative-falls-back"),
+ pytest.param("0", "3", True, id="zero-would-disable-retries"),
+ ],
+ )
+ def test_retries_env_var(self, value, expected, warns):
+ result = self._import_with(AIRFLOW_CLI_API_RETRIES=value)
+
+ assert result.returncode == 0, result.stderr
+ # An exact match also pins that the warning never reaches stdout.
+ assert result.stdout.strip() == expected
+ assert ("AIRFLOW_CLI_API_RETRIES" in result.stderr) is warns
+
+ def test_zero_wait_is_allowed(self):
+ """Only the retry count needs a floor of 1; waiting 0 seconds between
retries is valid."""
+ result = subprocess.run(
+ [sys.executable, "-c", "import airflowctl.api.client as c;
print(c.API_RETRY_WAIT_MIN)"],
+ env={**os.environ, "AIRFLOW_CLI_API_RETRY_WAIT_MIN": "0"},
+ check=False,
+ text=True,
+ capture_output=True,
+ )
+
+ assert result.returncode == 0, result.stderr
+ assert result.stdout.strip() == "0"
+ assert result.stderr == ""
