-
-
Notifications
You must be signed in to change notification settings - Fork 9.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add an Example for automatic retries to the Advanced Usage docs #6258
Add an Example for automatic retries to the Advanced Usage docs #6258
Conversation
d2e61c6
to
fe5a6d9
Compare
- While Requests doesn't automatically retry failures, this ability is a very common advanced use case in real world applications. - Although there's a mention of this ability on the HTTPAdapter class docs, it's a bit buried and not very specific. - It makes sense then to have an Example in the HTTPAdapter section of the Advanced Usage docs with a basic template for how this can be accomplished with Requests. - Open questions: -- is the urllib3.util.Retry import appropriate? I've seen some SO posts which import it from requests.adapters (even though that's just referring to urllib3)
fe5a6d9
to
4709399
Compare
@nateprewitt and @sethmlarson, apologies for the tag-summon (I find that a bit discourteous sometimes) but I think this is a pretty low-effort, low-risk, high-value docs change that folks would find useful, and I'm guessing it probably just got lost in the rest of the activity on this project. Do either of you (or any other maintainers) have any thoughts or feedback on the change in general, or the questions I posed above in the original post? Is this something you'd consider accepting into the docs? |
Hi @matthewarmand, thanks for checking in. This does look reasonable at first glance, it appears to have fallen off the review radar. I'll do a quick review this evening and let you know if anything needs to change. Otherwise, I'm alright with adding this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks @matthewarmand!
Bump requests from 2.31.0 to 2.32.3 Bumps requests from 2.31.0 to 2.32.0. Release notes Sourced from requests's releases. v2.32.0 2.32.0 (2024-05-20) 🐍 PYCON US 2024 EDITION 🐍 Security Fixed an issue where setting verify=False on the first request from a Session will cause subsequent requests to the same origin to also ignore cert verification, regardless of the value of verify. (GHSA-9wx4-h78v-vm56) Improvements verify=True now reuses a global SSLContext which should improve request time variance between first and subsequent requests. It should also minimize certificate load time on Windows systems when using a Python version built with OpenSSL 3.x. (#6667) Requests now supports optional use of character detection (chardet or charset_normalizer) when repackaged or vendored. This enables pip and other projects to minimize their vendoring surface area. The Response.text() and apparent_encoding APIs will default to utf-8 if neither library is present. (#6702) Bugfixes Fixed bug in length detection where emoji length was incorrectly calculated in the request content-length. (#6589) Fixed deserialization bug in JSONDecodeError. (#6629) Fixed bug where an extra leading / (path separator) could lead urllib3 to unnecessarily reparse the request URI. (#6644) Deprecations Requests has officially added support for CPython 3.12 (#6503) Requests has officially added support for PyPy 3.9 and 3.10 (#6641) Requests has officially dropped support for CPython 3.7 (#6642) Requests has officially dropped support for PyPy 3.7 and 3.8 (#6641) Documentation Various typo fixes and doc improvements. Packaging Requests has started adopting some modern packaging practices. The source files for the projects (formerly requests) is now located in src/requests in the Requests sdist. (#6506) Starting in Requests 2.33.0, Requests will migrate to a PEP 517 build system using hatchling. This should not impact the average user, but extremely old versions of packaging utilities may have issues with the new packaging format. New Contributors @matthewarmand made their first contribution in psf/requests#6258 @cpzt made their first contribution in psf/requests#6456 ... (truncated) Changelog Sourced from requests's changelog. 2.32.0 (2024-05-20) Security Fixed an issue where setting verify=False on the first request from a Session will cause subsequent requests to the same origin to also ignore cert verification, regardless of the value of verify. (GHSA-9wx4-h78v-vm56) Improvements verify=True now reuses a global SSLContext which should improve request time variance between first and subsequent requests. It should also minimize certificate load time on Windows systems when using a Python version built with OpenSSL 3.x. (#6667) Requests now supports optional use of character detection (chardet or charset_normalizer) when repackaged or vendored. This enables pip and other projects to minimize their vendoring surface area. The Response.text() and apparent_encoding APIs will default to utf-8 if neither library is present. (#6702) Bugfixes Fixed bug in length detection where emoji length was incorrectly calculated in the request content-length. (#6589) Fixed deserialization bug in JSONDecodeError. (#6629) Fixed bug where an extra leading / (path separator) could lead urllib3 to unnecessarily reparse the request URI. (#6644) Deprecations Requests has officially added support for CPython 3.12 (#6503) Requests has officially added support for PyPy 3.9 and 3.10 (#6641) Requests has officially dropped support for CPython 3.7 (#6642) Requests has officially dropped support for PyPy 3.7 and 3.8 (#6641) Documentation Various typo fixes and doc improvements. Packaging Requests has started adopting some modern packaging practices. The source files for the projects (formerly requests) is now located in src/requests in the Requests sdist. (#6506) Starting in Requests 2.33.0, Requests will migrate to a PEP 517 build system using hatchling. This should not impact the average user, but extremely old versions of packaging utilities may have issues with the new packaging format. Commits d6ebc4a v2.32.0 9a40d12 Avoid reloading root certificates to improve concurrent performance (#6667) 0c030f7 Merge pull request #6702 from nateprewitt/no_char_detection 555b870 Allow character detection dependencies to be optional in post-packaging steps d6dded3 Merge pull request #6700 from franekmagiera/update-redirect-to-invalid-uri-test bf24b7d Use an invalid URI that will not cause httpbin to throw 500 2d5f547 Pin 3.8 and 3.9 runners back to macos-13 (#6688) f1bb07d Merge pull request #6687 from psf/dependabot/github_actions/github/codeql-act... 60047ad Bump github/codeql-action from 3.24.0 to 3.25.0 31ebb81 Merge pull request #6682 from frenzymadness/pytest8 Additional commits viewable in compare view Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase. Dependabot commands and options You can trigger Dependabot actions by commenting on this PR: @dependabot rebase will rebase this PR @dependabot recreate will recreate this PR, overwriting any edits that have been made to it @dependabot merge will merge this PR after your CI passes on it @dependabot squash and merge will squash and merge this PR after your CI passes on it @dependabot cancel merge will cancel a previously requested merge and block automerging @dependabot reopen will reopen this PR if it is closed @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually @dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the Security Alerts page. Reviewed-by: Yustina Reviewed-by: Vladimir Vshivkov
While Requests doesn't automatically retry failures, this is an extremely common advanced use case in real world applications.
Although there's a mention of this capability in the
HTTPAdapter
API docs, it's a bit buried and not very specific.The ubiquity of the use case fits perfectly in the Transport Adapters section of the Advanced Usage docs, so I thought it would be useful to have a simple Example of this capability.
I took a stab at such an addition, and tried to follow the style of the rest of the docs pretty closely.
Linking over to the
urllib3.util.Retry
docs seems like it should reinforce the fact that Requests isn't the owner of that code and that library should be consulted with questions about that class (rather than Requests' support of it).I'm curious if maintainers have any feedback about the content or code example. Specifically I was curious whether you have any opinions as to the appropriate standard for the
Retry
import. Whileurllib3.util.Retry
is where the class lives and what theurllib3
docs reference,from urllib3 import Retry
also works, and in some StackOverflow posts I've seen people usefrom requests.adapters import Retry
. I felt that last one would telegraph an ownership by Requests that would be undesirable, but I'm wondering if anyone is particularly picky about the best practice for that import.Also, let me know if this is non-trivial enough to add a HISTORY entry for.