prkumar / uplink

A Declarative HTTP Client for Python
https://uplink.readthedocs.io/
MIT License
1.07k stars 61 forks source link

[Needs Feedback] Determine sensible defaults for @retry decorator #160

Open prkumar opened 5 years ago

prkumar commented 5 years ago

Is your feature request related to a problem? Please describe. @liiight recently encountered a problem that may indicate that the current defaults for the @retry decorator are not sensible. Below are excerpts of the problem he encountered, as detailed on Gitter:

... I recently had a pretty interesting issue involving uplinks exponential retry I'm using it as a way to handle some API pretty strict rate limit. I set the base to 2 and multiplier to 0.5 and all looked fine Cut to a few weeks later where my ci builds are starting to take hours longer (you can probably see where this is going...) So apparantly the retry value has gotten to be so high that it was more than an hour between retries Obviously that's by design as the max value is set to sys.maxsize Maybe setting that to that value by default isnt the best approach? As a user I'd rather have a default strict retry policy and loosen it up as needed than encounter an issue as I did

Describe the solution you'd like We need to have a discussion concerning what are sensible defaults for:

  1. The number of retry attempts (i.e., the default stop strategy).
    • Currently this is indefinite; we'll continue to retry until we get a successful response
  2. The maximum wait time:
    • Currently, this is equal to sys.maxint, for no particular reason.
liiight commented 5 years ago
  1. I'd say limiting to a reasonable number of retries (let's say 3) makes sense.
  2. Maybe 60 seconds would be reasonable here.

Like we discussed in Gitter, as a user I'd much rather have stricter defaults that will need to be loosened than vice versa. So the defaults themselves aren't that important, but the fact that they exist does.

Also worth mentioning that this could be potentially breaking some users behaviour that rely on the current defaults, may need to issue some note of this in change-log.