Add hooks for `@retry` decorator

[prkumar]

Is your feature request related to a problem? Please describe. This FR is from @liiight on gitter:

So i was trying to add metrics to get a good sense of our requests/rate limit issue and while i can add a metric to the response via a response_handler, I can not do the same per each retry. Maybe you should have a retry_handler that will be used for those requests.

Describe the solution you'd like

One way we could approach this is with a on_retry argument for the @retry decorator. This would be a function that is called when the retry condition is met, and it would get the most recent exception or failed response on each invocation:

@retry(..., on_retry=count_retries)
@get('users')
def get_users(self):
    pass

Alternatively, we could add the hook using a @retry.handler decorator:

@retry.handler(count_retries)
@retry(...)
@get('users')
def get_users(self):
    pass

[liiight]

@prkumar this issue is really problematic for me. Is there any way I can help with this? You think this would make a good issue to contribute to?

[prkumar]

@liiight I've added the Critical label to this issue and pinned it, so I'll prioritize the work for this one! If you'd like to tackle a solution, I'd suggest looking at the uplink/retry/retry.py module. Specifically, these lines:

https://github.com/prkumar/uplink/blob/ff72f3faceaf3a70feeffbfe49c73d5146d18a7b/uplink/retry/retry.py#L33-L45

For both method, the if case is where you want to invoke the new retry handler.

Possible Workaround

Here's a workaround that you can use right now: define your own uplink.retry.when.RetryPredicate.

For example, here's a very simple implementation for a retry counter:

from uplink.retry.when import RetryPredicate

class RetryPredicateWithCounter(RetryPredicate):
    def __init__(self, predicate):
        self._predicate = predicate  # This should be a `retry.when.*` predicate
        self._retry_count = 0

    def should_retry_after_response(self, response):
        should_retry = self._predicate.should_retry_after_response(response)
        self_retry_count = 0 if not should_retry else self_retry_count + 1
        return result

    def should_retry_after_exception(self, exc_type, exc_val, exc_tb):
        should_retry = self._predicate.should_retry_after_exception(exc_type, exc_val, exc_tb)
        self_retry_count = 0 if not should_retry else self_retry_count + 1
        return result

Then, you would pass a RetryPredicateWithCounter instance into the when argument for the uplink.retry decorator:

@retry(when=RetryPredicateWithCounter(retry.when.raises(MyException)))
@get('users')
def get_users(self):
    pass

[liiight]

Thanks for the workaround the pointers!

I'd like to run another design option other than those you suggested, more consistent with response handler (which i really like)

@uplink.retry_handler
def a_retry_handler(retry_exception, response):
	if response.status_code == 404:
		do_something()
	return response # maybe raise the retry exception?

@a_retry_handler
@retry(...)
@get('users')
def get_users(self):
    pass

WDYT?

[prkumar]

@liiight - The @retry_handler design looks good to me!

Have you had a chance to try out the workaround I mentioned in my previous comment? It should work with the latest version of the library.

[cognifloyd]

@retry_handler looks great to me. I tried creating a @response_handler to raise an exception to be caught by an @retry, but that failed because @retry runs before @response_handler. It took a few weeks for me to figure out that my retry was actually having no effect. So, making it easier to debug @retry, or making a simple interface for making your own retry logic (like @retry_handler) sounds great.

I ended up writing my own RetryPredicate:

class FutureIsNotCompleted(retry.when.RetryPredicate):
    # noinspection PyMethodMayBeStatic
    def should_retry_after_response(self, response):
        try:
            res_json = response.json()
        except ValueError:
            res_json = [{}]
        # status: Completed, Inprogress
        return not all(outer.get("status") == "Completed" for outer in res_json)

class FuturesMixin:  # meant to be inherited by a class that also inherits from uplink.Consumer
    @retry(when=FutureIsNotCompleted(), stop=retry.stop.after_delay(240))
    @returns_results_or_future
    @get(args=(Url,))
    def _check_async_results(self, location):
        pass