This is an automated email from the ASF dual-hosted git repository.
potiuk 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 1be286ecfe4 Fix a bad retry env var taking down the whole airflowctl
CLI (#72429)
1be286ecfe4 is described below
commit 1be286ecfe49f3be3ab842dff5cadba497d7ceaf
Author: Y-C <[email protected]>
AuthorDate: Tue Sep 8 02:36:27 2026 +0800
Fix a bad retry env var taking down the whole airflowctl CLI (#72429)
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.
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 51e92d81375..e0c9c60f0ee 100644
--- a/airflow-ctl/src/airflowctl/api/client.py
+++ b/airflow-ctl/src/airflowctl/api/client.py
@@ -325,10 +325,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 == ""
