Skip to content
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

Support user-defined retrying strategies #2

Open
jwodder opened this issue Dec 7, 2023 · 0 comments
Open

Support user-defined retrying strategies #2

jwodder opened this issue Dec 7, 2023 · 0 comments
Labels
breaking change Introduction of an incompatible API change enhancement New feature or request therefor under consideration Dev has not yet decided whether or how to implement

Comments

@jwodder
Copy link
Owner

jwodder commented Dec 7, 2023

Possible API:

  • The Client constructor is passed an instance of a Retrier protocol instead of a RetryConfig.

  • When the client gets an error (including an HTTPError from raise_for_status()), it passes the error to the retrier's handle() (or on_error()?) method along with a dataclass with attributes for retry number and the time at which the client started attemping to make the request before retries.

  • The handle method either returns a float/int — causing the client to sleep that many seconds and then retry — or else returns None — indicating that no retrying should occur, in which case the client reraises the original error.

  • The dataclass should also have a dict attribute for the retrier to store arbitrary data in

  • Also give the dataclass method and url attributes

  • Idea: Change Retrier to an ABC and give it an on_success() method that defaults to pass?

    • This can be used to emit a log message when a request succeeds after retries
  • The current retrying strategy should be implemented by a default Retrier class (named GitHubRetrier?).

  • Add a NullRetrier implementation that never retries

  • Ideally, it should be easy for the user to take the default retrier and add in a custom retry condition to consider in addition to the default conditions, possibly with calculating of custom sleep times when this condition occurs.

    • It should be easy for a custom retry condition to indicate that the sleep time should be based on exponential backoff without the condition having to do the backoff calculation itself.

      • Give the default retrier a backoff(attempts: int) -> float method for this purpose?
      • Technically, if a custom retry check were to return a delay of 0 or a negative number, this would result in the use of exponential backoff, but that may be too obtuse.
    • If a custom retry condition returns a sleep time less than the current exponential backoff time, the latter is used as the sleep time instead, just like for the current strategy

    • idea: Define the default retrier or another retrier as taking a collection of Callable[[RequestError, RetryState], float | None] values, and then the handle method calls each one and uses the maximum result (possibly clamped to not exceed the stop time)

Once this is done, use ghreq in tinuous.

@jwodder jwodder added enhancement New feature or request therefor under consideration Dev has not yet decided whether or how to implement breaking change Introduction of an incompatible API change labels Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Introduction of an incompatible API change enhancement New feature or request therefor under consideration Dev has not yet decided whether or how to implement
Projects
None yet
Development

No branches or pull requests

1 participant