Retry requests

[StephenBrown2]

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.

[lovelydinosaur]

I think our tack onto this should probably be to match requests API wherever possible (so no built-in retry functionality). What we should defiantly do tho would be to do a good job of documenting how to implement this or other functionality with a custom dispatcher. That way we can make sure we're allowing for a range of differing behaviors, without having to extend our API surface area.

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.

[sethmlarson]

I'm actually of the opposite opinion here. Every requests usage I've seen in the wild uses urllib3's Retry, let's not repeat that lapse of API. :)

[lovelydinosaur]

Oh right, I was under the impression that it disables it, but I must be misremembering?

[lovelydinosaur]

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

[lovelydinosaur]

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.

[Hellowlol]

I have used backoff atm, but it would be nice to have some native solution 👍

[StephenBrown2]

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.

[sethmlarson]

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.

[StephenBrown2]

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.

[lovelydinosaur]

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.

[sethmlarson]

I'll start by enumerating urllib3's existing functionality for Retry:

• read Number of times that a read operation can fail
• write Number of times that a write operation can fail
• connect Number of times that an operation related to creating a connection can fail (Doesn't apply to read timeouts on TLS, those are under read)
• redirect Number of times that a redirect can be followed
• status Number of times that we can retry a request based on the received responses status code being in the status_forcelist and the request method being in method_whitelist.
• Exponential backoff
• raise_on_redirect and raise_on_status returns whether or not we should raise an error or just return the response.
• respect_retry_after is whether a retry based on a response should respect he Retry-After header by sleeping.
• remove_headers_on_redirect is a set of headers that should be removed from subsequent requests when a redirect is issued.

[sethmlarson]

IMO the raise_on_redirect and raise_on_status are things that don't need to be attached to the Retry class?

[StephenBrown2]

Here's a typed skeleton of the current Retry object's public interface:
https://gist.github.com/StephenBrown2/2fc6bab18b30037488deb0f4db92e001