From dee8883b817938abd2783fb04ff0327143ef8aa6 Mon Sep 17 00:00:00 2001 From: Raj Kumar Date: Wed, 5 Jan 2022 19:02:51 -0800 Subject: [PATCH] Simplify backoff implementation --- tests/unit/test_retry.py | 18 +++++------ uplink/retry/backoff.py | 42 +++++++++++++------------- uplink/retry/retry.py | 64 +++++++++++----------------------------- 3 files changed, 49 insertions(+), 75 deletions(-) diff --git a/tests/unit/test_retry.py b/tests/unit/test_retry.py index 28ddbcc..0fd7b9e 100644 --- a/tests/unit/test_retry.py +++ b/tests/unit/test_retry.py @@ -38,21 +38,21 @@ def test_fixed_backoff(): def test_compose_backoff(mocker): left = backoff.from_iterable([0, 1]) right = backoff.from_iterable([2]) - mocker.spy(left, "after_stop") - mocker.spy(right, "after_stop") + mocker.spy(left, "handle_after_final_retry") + mocker.spy(right, "handle_after_final_retry") strategy = left | right # Should return None after both strategies are exhausted - assert strategy.after_response(None, None) == 0 - assert strategy.after_exception(None, None, None, None) == 1 - assert strategy.after_response(None, None) == 2 - assert strategy.after_exception(None, None, None, None) is None + assert strategy.get_timeout_after_response(None, None) == 0 + assert strategy.get_timeout_after_exception(None, None, None, None) == 1 + assert strategy.get_timeout_after_response(None, None) == 2 + assert strategy.get_timeout_after_exception(None, None, None, None) is None # Should invoke both strategies after_stop method - strategy.after_stop() + strategy.handle_after_final_retry() - left.after_stop.assert_called_once_with() - right.after_stop.assert_called_once_with() + left.handle_after_final_retry.assert_called_once_with() + right.handle_after_final_retry.assert_called_once_with() def test_retry_stop_default(): diff --git a/uplink/retry/backoff.py b/uplink/retry/backoff.py index 7e46758..07f9648 100644 --- a/uplink/retry/backoff.py +++ b/uplink/retry/backoff.py @@ -48,7 +48,7 @@ class RetryBackoff(object): return ``None``. """ - def after_response(self, request, response): + def get_timeout_after_response(self, request, response): """ Returns the number of seconds to wait before retrying the request, or ``None`` to indicate that the given response should @@ -56,7 +56,7 @@ def after_response(self, request, response): """ raise NotImplementedError # pragma: no cover - def after_exception(self, request, exc_type, exc_val, exc_tb): + def get_timeout_after_exception(self, request, exc_type, exc_val, exc_tb): """ Returns the number of seconds to wait before retrying the request, or ``None`` to indicate that the given exception @@ -64,7 +64,7 @@ def after_exception(self, request, exc_type, exc_val, exc_tb): """ raise NotImplementedError # pragma: no cover - def after_stop(self): + def handle_after_final_retry(self): """ Handles any clean-up necessary following the final retry attempt. @@ -84,23 +84,25 @@ def __init__(self, left, right): self._left = left self._right = right - def after_response(self, request, response): - delay = self._left.after_response(request, response) - if delay is None: - return self._right.after_response(request, response) - return delay - - def after_exception(self, request, exc_type, exc_val, exc_tb): - delay = self._left.after_exception(request, exc_type, exc_val, exc_tb) - if delay is None: - return self._right.after_exception( + def get_timeout_after_response(self, request, response): + timeout = self._left.get_timeout_after_response(request, response) + if timeout is None: + return self._right.get_timeout_after_response(request, response) + return timeout + + def get_timeout_after_exception(self, request, exc_type, exc_val, exc_tb): + timeout = self._left.get_timeout_after_exception( + request, exc_type, exc_val, exc_tb + ) + if timeout is None: + return self._right.get_timeout_after_exception( request, exc_type, exc_val, exc_tb ) - return delay + return timeout - def after_stop(self): - self._left.after_stop() - self._right.after_stop() + def handle_after_final_retry(self): + self._left.handle_after_final_retry() + self._right.handle_after_final_retry() class _IterableBackoff(RetryBackoff): @@ -121,13 +123,13 @@ def __next(self): except StopIteration: return None - def after_response(self, request, response): + def get_timeout_after_response(self, request, response): return self.__next() - def after_exception(self, request, exc_type, exc_val, exc_tb): + def get_timeout_after_exception(self, request, exc_type, exc_val, exc_tb): return self.__next() - def after_stop(self): + def handle_after_final_retry(self): self.__iterator = None diff --git a/uplink/retry/retry.py b/uplink/retry/retry.py index 6b38891..9f06a14 100644 --- a/uplink/retry/retry.py +++ b/uplink/retry/retry.py @@ -6,7 +6,6 @@ stop as stop_mod, backoff as backoff_mod, ) -from uplink.retry.backoff import RetryBackoff from uplink.retry._helpers import ClientExceptionProxy __all__ = ["retry"] @@ -107,69 +106,42 @@ def __init__( def modify_request(self, request_builder): request_builder.add_request_template( _RetryTemplate( - _RetryStrategy( - self._when(request_builder), - self._backoff, - self._stop, - ) + self._when(request_builder), + self._backoff, + self._stop, ) ) -class _RetryStrategy(RetryBackoff): +class _RetryTemplate(RequestTemplate): def __init__(self, condition, backoff, stop): self._condition = condition self._backoff = backoff self._stop = stop self._stop_iter = self._stop() - def _process_delay(self, delay): + def _process_timeout(self, timeout): next(self._stop_iter) - if self._stop_iter.send(delay): + if timeout is None or self._stop_iter.send(timeout): + self._backoff.handle_after_final_retry() + self._stop_iter = self._stop() return None - return delay + return transitions.sleep(timeout) def after_response(self, request, response): if not self._condition.should_retry_after_response(response): - return None - - delay = self._backoff.after_response(request, response) - return self._process_delay(delay) + return self._process_timeout(None) + return self._process_timeout( + self._backoff.get_timeout_after_response(request, response) + ) def after_exception(self, request, exc_type, exc_val, exc_tb): if not self._condition.should_retry_after_exception( exc_type, exc_val, exc_tb ): - return None - - delay = self._backoff.after_exception( - request, exc_type, exc_val, exc_tb - ) - return self._process_delay(delay) - - def after_stop(self): - self._backoff.after_stop() - self._stop_iter = self._stop() - - -class _RetryTemplate(RequestTemplate): - def __init__(self, strategy): - self._strategy = strategy - - def after_response(self, request, response): - delay = self._strategy.after_response(request, response) - if delay is None: - self._strategy.after_stop() - return - - return transitions.sleep(delay) - - def after_exception(self, request, exc_type, exc_val, exc_tb): - delay = self._strategy.after_exception( - request, exc_type, exc_val, exc_tb + return self._process_timeout(None) + return self._process_timeout( + self._backoff.get_timeout_after_exception( + request, exc_type, exc_val, exc_tb + ) ) - if delay is None: - self._strategy.after_stop() - return - - return transitions.sleep(delay)