[DEV-12952] Add Configurable Retry Behavior

indico/config/config.py

@@ -16,6 +16,10 @@ class IndicoConfig:
         api_token= (str, optional): The actual text of the API Token. Takes precedence over api_token_path
         verify_ssl= (bool, optional): Whether to verify the host's SSL certificate. Default=True
         requests_params= (dict, optional): Dictionary of requests. Session parameters to set
+        retry_count= (int, optional): Retry API calls this many times.
+        retry_wait= (float, optional): Wait this many seconds after the first error before retrying.
+        retry_backoff= (float, optional): Multiply the wait time by this amount for each additional error.
+        retry_jitter= (float, optional): Add a random amount of time (up to this percent as a decimal) to the wait time to prevent simultaneous retries.
 
     Returns:
         IndicoConfig object
@@ -33,6 +37,11 @@ class IndicoConfig:
     requests_params: dict = None
     _disable_cookie_domain: bool = False
 
+    retry_count: int = 4
+    retry_wait: float = 1
+    retry_backoff: float = 4
+    retry_jitter: float = 0.5
+
     def __init__(self, **kwargs):
 
         self.host: str = os.getenv("INDICO_HOST")

indico/http/client.py

@@ -18,7 +18,7 @@
 )
 from indico.http.serialization import aio_deserialize, deserialize
 
-from .retry import aioretry
+from .retry import retry
 
 logger = logging.getLogger(__file__)
 
@@ -41,6 +41,14 @@ class HTTPClient:
     def __init__(self, config: IndicoConfig = None):
         self.config = config
         self.base_url = f"{self.config.protocol}://{self.config.host}"
+        self._decorate_with_retry = retry(
+            requests.RequestException,
+            count=self.config.retry_count,
+            wait=self.config.retry_wait,
+            backoff=self.config.retry_backoff,
+            jitter=self.config.retry_jitter,
+        )
+        self._make_request = self._decorate_with_retry(self._make_request)
 
         self.request_session = requests.Session()
         if config and isinstance(config.requests_params, dict):
@@ -150,6 +158,7 @@ def _make_request(
                 f"{self.base_url}{path}",
                 headers=headers,
                 stream=True,
+                timeout=(4, 64),
                 verify=False
                 if not self.config.verify_ssl or not self.request_session.verify
                 else True,
@@ -210,6 +219,14 @@ def __init__(self, config: Optional[IndicoConfig] = None):
         """
         self.config = config or IndicoConfig()
         self.base_url = f"{self.config.protocol}://{self.config.host}"
+        self._decorate_with_retry = retry(
+            aiohttp.ClientConnectionError,
+            count=self.config.retry_count,
+            wait=self.config.retry_wait,
+            backoff=self.config.retry_backoff,
+            jitter=self.config.retry_jitter,
+        )
+        self._make_request = self._decorate_with_retry(self._make_request)
 
         self.request_session = aiohttp.ClientSession()
         if config and isinstance(config.requests_params, dict):
@@ -282,7 +299,6 @@ def _handle_files(self, req_kwargs):
         if files:
             [f.close() for f in files]
 
-    @aioretry((aiohttp.ClientConnectionError, aiohttp.ServerDisconnectedError))
     async def _make_request(
         self,
         method: str,
@@ -311,6 +327,7 @@ async def _make_request(
             async with getattr(self.request_session, method)(
                 f"{self.base_url}{path}",
                 headers=headers,
+                timeout=aiohttp.ClientTimeout(sock_connect=4, sock_read=64),
                 verify_ssl=self.config.verify_ssl,
                 **request_kwargs,
             ) as response:

indico/http/retry.py

@@ -1,77 +1,93 @@
 import asyncio
-from random import randint
 import time
-import typing as t
 from functools import wraps
+from inspect import iscoroutinefunction
+from random import random
+from typing import TYPE_CHECKING, overload
 
+if TYPE_CHECKING:
+    from collections.abc import Awaitable, Callable
+    from typing import ParamSpec, TypeVar
+
+    ArgumentsType = ParamSpec("ArgumentsType")
+    OuterReturnType = TypeVar("OuterReturnType")
+    InnerReturnType = TypeVar("InnerReturnType")
 
 
 def retry(
-    ExceptionTypes: t.Type[Exception], tries: int = 3, delay: int = 1, backoff: int = 2
-) -> t.Callable:
+    *errors: "type[Exception]",
+    count: int = 4,
+    wait: float = 1,
+    backoff: float = 4,
+    jitter: float = 0.5,
+) -> "Callable[[Callable[ArgumentsType, OuterReturnType]], Callable[ArgumentsType, OuterReturnType]]":  # noqa: E501
     """
-    Retry with exponential backoff
+    Decorate a function or coroutine to retry when it raises specified errors,
+    apply exponential backoff and jitter to the wait time,
+    and raise the last error if it retries too many times.
+
+    By default, the decorated function or coroutine will be retried up to 4 times over
+    the course of ~2 minutes (waiting 1, 4, 16, and 64 seconds; plus up to 50% jitter).
 
-    Original from: http://wiki.python.org/moin/PythonDecoratorLibrary#Retry
+    Arguments:
+        errors:  Retry the function when it raises one of these errors.
+        count:   Retry the function this many times.
+        wait:    Wait this many seconds after the first error before retrying.
+        backoff: Multiply the wait time by this amount for each additional error.
+        jitter:  Add a random amount of time (up to this percent as a decimal)
+                 to the wait time to prevent simultaneous retries.
     """
 
-    def retry_decorator(f: t.Callable) -> t.Any:
-        @wraps(f)
-        def retry_fn(*args: t.Any, **kwargs: t.Any) -> t.Any:
-            n_tries, n_delay = tries, delay
-            while n_tries > 1:
-                try:
-                    return f(*args, **kwargs)
-                except ExceptionTypes:
-                    time.sleep(n_delay)
-                    n_tries -= 1
-                    n_delay *= backoff
-            return f(*args, **kwargs)
+    def wait_time(times_retried: int) -> float:
+        """
+        Calculate the sleep time based on number of times retried.
+        """
+        return wait * backoff**times_retried * (1 + jitter * random())
 
-        return retry_fn
+    @overload
+    def retry_decorator(
+        decorated: "Callable[ArgumentsType, Awaitable[InnerReturnType]]",
+    ) -> "Callable[ArgumentsType, Awaitable[InnerReturnType]]": ...
+    @overload
+    def retry_decorator(
+        decorated: "Callable[ArgumentsType, InnerReturnType]",
+    ) -> "Callable[ArgumentsType, InnerReturnType]": ...
+    def retry_decorator(
+        decorated: "Callable[ArgumentsType, InnerReturnType]",
+    ) -> "Callable[ArgumentsType, Awaitable[InnerReturnType]] | Callable[ArgumentsType, InnerReturnType]":  # noqa: E501
+        """
+        Decorate either a function or coroutine as appropriate.
+        """
+        if iscoroutinefunction(decorated):
+            @wraps(decorated)
+            async def retrying_coroutine(  # type: ignore[return]
+                *args: "ArgumentsType.args", **kwargs: "ArgumentsType.kwargs"
+            ) -> "InnerReturnType":
+                for times_retried in range(count + 1):
+                    try:
+                        return await decorated(*args, **kwargs)  # type: ignore[no-any-return]
+                    except errors:
+                        if times_retried >= count:
+                            raise
 
-    return retry_decorator
+                    await asyncio.sleep(wait_time(times_retried))
 
-def aioretry(
-    ExceptionTypes: t.Type[Exception],
-    tries: int = 3,
-    delay: t.Union[int, t.Tuple[int, int]] = 1,
-    backoff: int = 2,
-    condition: t.Optional[t.Callable[[Exception], bool]] = None,
-) -> t.Callable:
-    """
-    Retry with exponential backoff
+            return retrying_coroutine
 
-    Original from: http://wiki.python.org/moin/PythonDecoratorLibrary#Retry
-    Options:
-        condition: Callable to evaluate if an exception of a given type
-            is retryable for additional handling
-        delay: an initial time to wait (seconds). If a tuple, choose a random number
-            in that range to start. This can helps prevent retries at the exact
-            same time across multiple concurrent function calls
-    """
+        else:
+            @wraps(decorated)
+            def retrying_function(  # type: ignore[return]
+                *args: "ArgumentsType.args", **kwargs: "ArgumentsType.kwargs"
+            ) -> "InnerReturnType":
+                for times_retried in range(count + 1):
+                    try:
+                        return decorated(*args, **kwargs)
+                    except errors:
+                        if times_retried >= count:
+                            raise
 
-    def retry_decorator(f: t.Callable) -> t.Callable:
-        @wraps(f)
-        async def retry_fn(*args: t.Any, **kwargs: t.Any) -> t.Any:
-            n_tries = tries
-            if isinstance(delay, tuple):
-                # pick a random number to sleep
-                n_delay = randint(*delay)
-            else:
-                n_delay = delay
-            while True:
-                try:
-                    return await f(*args, **kwargs)
-                except ExceptionTypes as e:
-                    if condition and not condition(e):
-                        raise
-                    await asyncio.sleep(n_delay)
-                    n_tries -= 1
-                    n_delay *= backoff
-                    if n_tries <= 0:
-                        raise
+                    time.sleep(wait_time(times_retried))
 
-        return retry_fn
+            return retrying_function
 
     return retry_decorator

tests/unit/http/test_retry.py

@@ -0,0 +1,73 @@
+import pytest
+
+from indico.http.retry import retry
+
+
+def test_no_errors() -> None:
+    @retry(Exception)
+    def no_errors() -> bool:
+        return True
+
+    assert no_errors()
+
+
+def test_raises_errors() -> None:
+    calls = 0
+
+    @retry(RuntimeError, count=4, wait=0)
+    def raises_errors() -> None:
+        nonlocal calls
+        calls += 1
+        raise RuntimeError()
+
+    with pytest.raises(RuntimeError):
+        raises_errors()
+
+    assert calls == 5
+
+
+def test_raises_other_errors() -> None:
+    calls = 0
+
+    @retry(RuntimeError, count=4, wait=0)
+    def raises_errors() -> None:
+        nonlocal calls
+        calls += 1
+        raise ValueError()
+
+    with pytest.raises(ValueError):
+        raises_errors()
+
+    assert calls == 1
+
+
+@pytest.mark.asyncio
+async def test_raises_errors_async() -> None:
+    calls = 0
+
+    @retry(RuntimeError, count=4, wait=0)
+    async def raises_errors() -> None:
+        nonlocal calls
+        calls += 1
+        raise RuntimeError()
+
+    with pytest.raises(RuntimeError):
+        await raises_errors()
+
+    assert calls == 5
+
+
+@pytest.mark.asyncio
+async def test_raises_other_errors_async() -> None:
+    calls = 0
+
+    @retry(RuntimeError, count=4, wait=0)
+    async def raises_errors() -> None:
+        nonlocal calls
+        calls += 1
+        raise ValueError()
+
+    with pytest.raises(ValueError):
+        await raises_errors()
+
+    assert calls == 1