Eason09053360 opened a new pull request, #72708:
URL: https://github.com/apache/airflow/pull/72708

   `airflow db check --retry N` logs a countdown between attempts that is off 
by one, so the last
   message before the final attempt contradicts itself:
   
   ```
   $ airflow db check --retry 3 --retry-delay 9
   3 retries remain. Will retry in 9 seconds     <- was "2 retries remain"
   2 retries remain. Will retry in 9 seconds     <- was "1 retries remain"
   1 retries remain. Will retry in 9 seconds     <- was "0 retries remain. Will 
retry in 9 seconds"
   <final attempt, then the OperationalError is re-raised>
   ```
   
   The loop runs `stop_after_attempt(1 + retries)`, and tenacity's 
`before_sleep` hook fires only when
   another attempt will follow, with `attempt_number` being the attempt that 
has just failed. So after
   attempt *n* there are `(1 + retries) - n` attempts left. The callback 
computed
   `retries - attempt_number`, one lower, and the loop's `+ 1` was never 
mirrored in the message.
   
   The visible consequence is the final line: `0 retries remain. Will retry in 
9 seconds` — it
   announces that the command has given up at the exact moment it is about to 
try again.
   
   ## Impact
   
   This is the message people read while a container waits for its database. 
`airflow db check` is what
   `scripts/docker/entrypoint_prod.sh` runs (via `wait_for_airflow_db`) before 
the rest of the
   entrypoint proceeds, so the countdown is often the only feedback during a 
slow database start. A
   countdown that reaches zero and keeps going makes it look like the wait loop 
is stuck rather than
   progressing, which is exactly the wrong signal to send someone debugging a 
stalled deployment.
   
   ## Changes
   
   One line: `remain = retries - retrystate.attempt_number + 1`.
   
   ## Tests
   
   `test_check_warns_about_the_retries_that_are_actually_left` drives `db check 
--retry 3` with the
   database check patched to always fail, and asserts on the captured log via 
the `caplog` fixture:
   
   ```python
   assert "3 retries remain. Will retry in 9 seconds" in caplog
   assert "2 retries remain. Will retry in 9 seconds" in caplog
   assert "1 retries remain. Will retry in 9 seconds" in caplog
   assert "0 retries remain. Will retry in 9 seconds" not in caplog
   ```
   
   The final negative assertion pins the defect itself — the self-contradictory 
line must not appear.
   
   Asserting on the captured log rather than on the arguments handed to a 
mocked logger also covers the
   rendering: the module uses `structlog`, so this confirms the `%d` 
placeholders are actually
   interpolated and not printed literally.
   
   Reverting the one-line change and running the whole file fails this test; 
with the change, all 118
   tests in the file pass. `--retry 0` is unaffected: `before_sleep` never 
fires, so no countdown is
   logged at all.
   
   ## Notes for reviewers
   
   The corrected countdown now ends at `1`, so the message reads `1 retries 
remain`. The awkward plural
   is pre-existing wording that this change does not touch; happy to fix it 
here if preferred.
   
   ---
   
   ##### 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]

Reply via email to