fix: read the tornado global live instead of an import-time snapshot - #695
Closed
jd wants to merge 2 commits into
Closed
Conversation
Three runtime regressions from the mypy-strictness stack (#679, #677), all A/B verified against a2af454. None were caught by the test suite, and all three only fire once a retry actually happens, so happy-path callers see nothing until production. 1. `Retrying(wait=None)`, `wait=0` and `wait=sum([])` now raise `TypeError: 'NoneType' object is not callable` from inside `iter()`. #679 removed the `if self.wait:` guard on the grounds that `truthy-bool` proved it always true -- but that only holds for the *declared* type. Untyped callers pass `None`/`0` to mean "no wait", and `sum([])` over an empty strategy list returns the int 0. The `TypeError` is raised outside the attempt's try/except, so it escapes uncaught and destroys the caller's real exception. Restore the guard with a local `truthy-bool` suppression explaining why the "impossible" branch is reachable. `before`, `after`, `before_sleep` and `retry_error_callback` all kept their `is not None` guards; `wait` was the only one dropped. 2. `plain_callable + wait_strategy` now raises `TypeError`. `WaitBaseT` admits plain callables, and a function has no `__add__`, so this went through `wait_base.__radd__` -- which #677 narrowed to `int`. Widen it to `WaitBaseT | int` and build the `wait_combine` again. `5 + strategy` is still rejected, now via `NotImplemented` rather than a combination that fails later. The parameter stays `int` rather than `Literal[0]` because typeshed's `sum()` protocol requires `__radd__(x: int)`; narrowing it would make every `sum()` over strategies need a `type: ignore`. 3. `wait_combine.__call__` passed the state as `retry_state=`, which crashes on any `WaitBaseT` callable whose parameter has another name. Pass it positionally, as `BaseRetrying._run_wait` already does. Adds regression tests for all three plus the async path -- the original change shipped with no test covering any of them. Change-Id: Ia40c32a22cdac09cbfba4280fb0a7f0c2cb7b7e0
#679 replaced `if tornado:` with a `_HAS_TORNADO` bool computed once at import, to stop `truthy-bool` flagging the module object as always true. But only one of the two call sites was converted: the `retry()` decorator still dereferences the live `tornado` global right after testing the snapshot. The two desync as soon as anything reassigns the global. A/B verified against a2af454: `tenacity.tornado = None` followed by any `@tenacity.retry` decoration returned normally before, and now raises `AttributeError: 'NoneType' object has no attribute 'gen'`. Nulling the module global is the standard way a downstream suite exercises the non-tornado path without uninstalling tornado. Test the availability of the live global at the point of use, and add a regression test. `_HAS_TORNADO` is kept for the module-bottom import guard, where snapshot and global cannot diverge. Change-Id: Ia5592ff6a59cdc4c649ce26fff0e5938163544c1
Owner
Author
|
This pull request is part of a Mergify stack:
|
Base automatically changed from
devs/jd/fix/wait-regressions/restore-support-falsy-wait-values-callable--a40c32a2
to
main
August 6, 2026 06:44
jd
commented
Aug 6, 2026
jd
deleted the
devs/jd/fix/wait-regressions/read-tornado-global-live-instead-import-time--a5592ff6
branch
August 6, 2026 06:47
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#679 replaced
if tornado:with a_HAS_TORNADObool computed once atimport, to stop
truthy-boolflagging the module object as always true.But only one of the two call sites was converted: the
retry()decoratorstill dereferences the live
tornadoglobal right after testing thesnapshot. The two desync as soon as anything reassigns the global.
A/B verified against a2af454:
tenacity.tornado = Nonefollowed by any@tenacity.retrydecoration returned normally before, and now raisesAttributeError: 'NoneType' object has no attribute 'gen'. Nulling themodule global is the standard way a downstream suite exercises the
non-tornado path without uninstalling tornado.
Test the availability of the live global at the point of use, and add a
regression test.
_HAS_TORNADOis kept for the module-bottom import guard,where snapshot and global cannot diverge.
Depends-On: #694