-
-
Notifications
You must be signed in to change notification settings - Fork 845
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
Retry requests #108
Comments
I think our tack onto this should probably be to match I'm not absolute on this one, so perhaps could reassess it at a later date, but for the time being I'd like to treat it as out of scope. |
I'm actually of the opposite opinion here. Every |
Oh right, I was under the impression that it disables it, but I must be misremembering? |
Okay, looks like it's disabled by default, but can be enabled in the HTTPAdapter API... https://github.com/kennethreitz/requests/blob/master/requests/adapters.py#L113 |
Not documented in the QuickStart or Advanced guides there, but it is part of the API reference. https://2.python-requests.org/en/master/api/#requests.adapters.HTTPAdapter So, sure, let's treat that as in-scope. |
I have used backoff atm, but it would be nice to have some native solution 👍 |
Should the urllib3 team be pinged on implementation details? I could potentially work on porting it straight over, but I'm unsure about where it would lie in this library, and if someone else knows better on how it would interact with Sync vs Async they would probably be better to implement. |
I'd love to be put on the review if you're willing to take a stab at implementation. :) No worries on getting it right the first time. |
Also want to give attribution where it's due, so would probably start with a copy and reference comment, then work on async-ifying it... Will put some effort into it over the next couple weeks. I don't have much free time, so I'm not opposed to duplicate work on it. |
It might be worth tackling it API-first, before jumping in on the implementation. How many controls and dials do we really want the retry API to provide? What critical bits of control over that are actually needed/used in the wild? Whatever is the most minimal possible sounds good to me - we can always extend it out further in time if needed. We may also want to think about allowing for more complex cases through customization rather than configuration. Eg. only provide for specifying "allow N retries", but also provide an implementation hook that folks can build more complex cases off of. That way you keep the API surface area nice and low, while still allowing flexiblity, or third party packages. |
I'll start by enumerating urllib3's existing functionality for
|
IMO the |
Here's a typed skeleton of the current Retry object's public interface: |
So I definitely want to make the new Retry object have one critical feature that actually makes sub-classing useful: Have a method that is subclassable that passes in the Request and the Response and then by some interface is allowed to decide whether a retry occurs and in addition also allows modifying the Request that will be emitted for that retry. This exact functionality is tough to implement in urllib3 because we don't allow you to modify headers and we don't give the user everything they might want to know (such as the whole request that was sent) Doing this allows so many things to be implemented simply by sub-classing the Retry. It's pretty critical actually that we get this interface right because there's a lot of functionality related to HTTP that involves multiple requests (Authentication, stripping headers on redirect, retry-after, resumable uploads, caching) that users have been panging for but is tough to implement without the right interface. |
I'd rather Retry was an ABC and could be implemented without subclassing |
Could you explain the benefit of not needing to implement with sub-classing in this situation? I'm not seeing one right away. |
There’s some great stuff to dig into here, tho I don’t think it’s on the critical path. The API will be Finer grained control and/or a method override will then exist on the RetryConfig. |
Personally, I don't see a need to support the What I would like to see, as the minimum for my needs right now, is:
https://pypi.org/project/backoff/ as mentioned by @Hellowlol might be something to look at for inspiration as well. |
Noting here that having And I'm of the opinion that having great defaults for the |
100%. No reason folks should have to think about this. |
How about just a |
I'm okay with having |
Since |
Per #134 we're going to hold off on implementing this feature. |
I was looking into this retry feature to be available in httpx (use case is to retry based on certain http status codes). but it seems like this feature is still not supported yet (or i am trying to do something wrong). Just wanted to confirm if this feature is supported already? and if not, when can we expect this feature to be available. |
I implemented retries in a HTTPTransport subclass recently. Whilst I can by no means claim this is good code, nor even that it's the right approach, nor that it would work for anything other than my specific use case, I thought sharing it might at least gives others a possible starting point: https://gitlab.com/openid/conformance-suite/-/blob/master/scripts/conformance.py#L19 Above is for synchronous requests. There's an async version here: https://gitlab.com/openid/conformance-suite/-/blob/httpx-async/scripts/conformance.py#L19 But I've not been able to get async requests to work reliably for me (for reasons currently unknown), so I'm currently only using the sync version. (I'm still trying to get to the bottom of various weird things going on, e.g. #2056 ) (Critiques of the code are very welcome. The suggestion of using 'backoff' above might've been a better approach, sadly I didn't notice that suggestion before I went this way.) |
I would love attention to be brought back to this feature implementation as time permits. |
Is there any recommended way to implement a retry mechanism with |
Would love to see this functionality in httpx. Started switching to the library and was disappointed by the lack of this feature or a suggested workaround, seems like a pretty common feature you would expect from a modern HTTP lib. |
So... "retries" in the context of HTTP could describe several different type of use-case. The valuable thing to do here would be to describe very specifically what behaviour you're trying to deal with. Talking through an actual "here's my specific problem" use-case, will help move the conversation forward.
|
use case |
here's a retry http transport wrapper, inspired partially by urlib3.util.Retry
|
Like @matt-mercer, I think this feature—even in limited form—would be incredibly valuable |
This would be a great feature to have (at least optionally), thanks @matt-mercer! I tried to to implement and came across the following to issues:
|
An improved version of what was proposed by @matt-mercer
|
There is actually the AsyncHTTPTransport class, which has the 'retry' parameter. Its not documented explicitly, but you can see its usage at https://www.python-httpx.org/async/ under subsection "Explicit transport instances". AsyncHTTPTransport is implemented in the httpcore package in connection class: https://github.com/encode/httpcore/blob/master/httpcore/_async/connection.py I am using this to check on failed connection, not on bad HTTP satus. The only thing it misses to set the retry delays explicitly. It has an exponential backoff time implemented. Might be nice to be able to control the delay time in the future. |
That is for retrying failed connections only. It does not retry on bad HTTP status codes as far as I can tell. |
Thanks for pointing that out! |
is there a plan to add the RetryTransport behaviour to the project ? I can send a PR for it. |
If someone if motivated to provide this functionality, then I'd suggest a third party package... https://www.python-httpx.org/third_party_packages/ |
urllib3 has a very convenient Retry utility (docs) that I have found to be quite useful when dealing with flaky APIs. http3's Clients don't support this sort of thing yet, but I would love it if they did!
In the meantime, I can probably work out my own with a while loop checking the response code.
The text was updated successfully, but these errors were encountered: