Eason09053360 opened a new pull request, #72429: URL: https://github.com/apache/airflow/pull/72429
`airflowctl`'s three retry knobs are coerced with `int()` at module scope, and the import chain `__main__ → cli_parser → cli_config → api.client` is unconditional. 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 `airflowctl --help` to find out what they had broken. A negative wait was worse. It parsed cleanly, so nothing complained at startup; `wait_random_exponential` then returns `random.uniform(self.min, high)` with no `max(0, …)` guard, and the resulting negative sleep raises out of tenacity on a random subset of retries. Measured on the pinned tenacity: 8 of 10 seeds produce a negative first wait, so the command fails intermittently, at request time, with an error that points nowhere near the cause. `_get_int_env` now parses defensively and range-checks, warning on stderr and falling back to the default: - `AIRFLOW_CLI_API_RETRIES` needs `>= 1`; `0` would leave `stop_after_attempt(0)` reraising on the first failure, silently disabling the retries the variable exists to configure. - The two wait knobs need `>= 0`. Zero is deliberately still accepted — retrying immediately is a legitimate choice, and only negatives reach `time.sleep()`. - An empty value is treated as unset rather than as a typo, since `ENV VAR=` is how Docker and Kubernetes spell "not configured"; warning there would mean permanent stderr noise on every invocation. The warning goes to `sys.stderr` (matching `client.py:141,144`) rather than the logger, because this runs at import time before logging is configured and `structlog`'s default sink is stdout — a warning there would corrupt `airflowctl pools list -o json > pools.json`. Two knowingly out-of-scope items: `WAIT_MIN` greater than `WAIT_MAX` still inverts into a long fixed sleep (each value is individually valid, so catching it needs cross-knob validation), and the same `int(os.getenv(...))` shape exists in the Task SDK client. Tests run each case in a fresh interpreter, because the values are read at import and a single process can only import the module once. Two cases (`7`, and `0` for the wait knob) pass against the unfixed code by design: they pin that the new guard does not over-reject, which nothing else covers — without them a helper that always returned the default would pass. --- ##### Was generative AI tooling used to co-author this PR? - [X] Yes — Claude Code (Opus 5) Generated-by: Claude Code (Opus 5) following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
