docs: describe what RetryHandlerOption.delay and max_delay actually do - #729
Vincent Biret (baywet) merged 4 commits into
Conversation
The option stores delay as max_delay and the handler adds it to every backoff; the caps are backoff_max and MAX_DELAY. The RetryHandler class docstring listed four constructor parameters that do not exist.
Vincent Biret (baywet)
left a comment
Vincent Biret (baywet)
left a comment
There was a problem hiding this comment.
Thanks for the contribution!
There was a problem hiding this comment.
🟡 Changes recommended
The updated documentation introduces/retains a few inaccuracies versus the current implementation (notably Retry-After: 0 handling and some wording around MAX_DELAY) that should be corrected before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Lite
Head branch was pushed to by a user without write access
Vincent Biret (baywet)
left a comment
Vincent Biret (baywet)
left a comment
There was a problem hiding this comment.
Thank you for making the changes!
There was a problem hiding this comment.
🟡 Changes recommended
One updated docstring still inaccurately describes the backoff cap as a fixed constant rather than the configurable backoff_max actually used by the implementation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
Head branch was pushed to by a user without write access
|
There was a problem hiding this comment.
🟢 Approval recommended
The changes are documentation-only and the updated docstrings match the current implementation behavior (retry conditions and delay computation).
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
Overview
Docstrings only, no behaviour change.
RetryHandlerOptionnow says what its constructor takes:delayis a base delay added to every computed backoff, exposed asmax_delay, and not a cap;max_retriesis exposed asmax_retry. The comment onMAX_DELAYsays it is the largest accepted value and the point at which the handler stops retrying. TheRetryHandlerclass docstring, which documented four parameters the constructor does not take, now describes the actual policy: status codes, methods, the option fields, and how the delay is computed.get_delay_timesays that aRetry-Afterheader is returned as is and that the backoff path is capped atbackoff_max.Related Issue
Fixes #728
Notes
respect_retry_after_header(retry_handler.py:66) is still assigned and never read; left alone here since this PR is docs only. Renamingmax_delaywould break a public property, so the name stays and the docstrings explain it.Testing Instructions
cd packages/http/httpx && pytest tests/middleware_tests/test_retry_handler.py: unchanged, passes.