fix(ndb): use the single-argument generator.throw() signature - #18159
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request simplifies exception propagation in tasklets by replacing the legacy three-argument generator.throw() call with the single-argument version generator.throw(error). This avoids DeprecationWarning in newer Python versions while preserving the traceback. A corresponding regression test has been added to verify this behavior and ensure no deprecation warnings are raised. There are no review comments, and no further feedback is required.
The three-argument form deprecated in Python 3.12 emits a DeprecationWarning for every exception crossing a tasklet boundary, and the docs say it may be removed in a future version, which would turn each of those into a TypeError. The single-argument form reads the traceback off the exception itself, so the local becomes redundant and the delivered exception is unchanged.
78e0ce2 to
4abc826
Compare
Fixes #18158
Summary
_TaskletFuture._advance_taskletthrows exceptions into the wrapped generator using the three-argument form ofgenerator.throw(), deprecated in Python 3.12:Every exception that crosses a tasklet boundary emits a
DeprecationWarning. This library's own unit suite raises 70 warnings today; with this change it raises 36, so 34 of them came from this single line.Today that is only noise, but the Python docs say the old signature "may be removed in a future version".
noxfile.pyalready lists3.15inALL_INTERPRETERS, so it is worth landing before that removal makes every tasklet-boundary exception raiseTypeErrorinstead of propagating.Changes
The single-argument form reads the traceback off the exception itself, so the local becomes redundant:
Also adds a regression test asserting both halves of the contract: the exception still arrives with its
__traceback__intact, and noDeprecationWarningis emitted. I verified the test fails against the old line and passes against the new one, so it genuinely guards the behaviour rather than just tracking it.Verification
pytest tests/unit— 1833 passed, 1 skipped (1832 before, plus the new test); warnings drop from 70 to 36ruff format --checkandflake8 google tests— cleangoogle-cloud-ndband the standard library; see google-cloud-ndb: DeprecationWarning from the three-arg generator.throw() in tasklets #18158 for the standalone snippet