Exponential Backoff Followup (#136)

Co-authored-by: github-actions <github-actions@github.com>

.github/workflows/wipac-cicd.yml

@@ -76,7 +76,7 @@ jobs:
           pip install .[tests]
       - name: Run Tests
         run: |
-          python -m pytest tests --tb=short --log-level=INFO -vvv
+          python -m pytest tests --tb=short --log-level=DEBUG -vvv
           pycycle --here --verbose
 
   py-tests-telemetry:
@@ -97,7 +97,7 @@ jobs:
           pip install .[telemetry,tests]
       - name: Run Tests
         run: |
-          python -m pytest tests --tb=short --log-level=INFO
+          python -m pytest tests --tb=short --log-level=DEBUG
           pycycle --here --verbose
       - name: Run Integration Test
         run: |

dependencies-dev.log

@@ -26,7 +26,7 @@ iniconfig==2.0.0
     # via pytest
 mccabe==0.7.0
     # via flake8
-mypy==1.4.1
+mypy==1.5.1
     # via wipac-rest-tools (setup.py)
 mypy-extensions==1.0.0
     # via mypy
@@ -65,11 +65,11 @@ requests-futures==1.0.1
     # via wipac-rest-tools (setup.py)
 requests-mock==1.11.0
     # via wipac-rest-tools (setup.py)
-ruff==0.0.282
+ruff==0.0.285
     # via wipac-rest-tools (setup.py)
 six==1.16.0
     # via requests-mock
-tornado==6.3.2
+tornado==6.3.3
     # via wipac-rest-tools (setup.py)
 types-requests==2.31.0.2
     # via wipac-rest-tools (setup.py)
@@ -82,7 +82,7 @@ typing-extensions==4.7.1
     #   wipac-dev-tools
 urllib3==2.0.4
     # via requests
-wheel==0.41.1
+wheel==0.41.2
     # via wipac-rest-tools (setup.py)
 wipac-dev-tools==1.6.16
     # via wipac-rest-tools (setup.py)

dependencies-mypy.log

@@ -14,7 +14,7 @@ cffi==1.15.1
     # via cryptography
 charset-normalizer==3.2.0
     # via requests
-click==8.1.6
+click==8.1.7
     # via
     #   click-completion
     #   pycycle
@@ -24,7 +24,7 @@ colorama==0.4.6
     # via crayons
 coloredlogs==15.0.1
     # via wipac-telemetry
-coverage[toml]==7.2.7
+coverage[toml]==7.3.0
     # via
     #   pytest-cov
     #   wipac-rest-tools (setup.py)
@@ -44,7 +44,7 @@ googleapis-common-protos==1.56.2
     # via
     #   opentelemetry-exporter-jaeger-proto-grpc
     #   opentelemetry-exporter-otlp-proto-http
-grpcio==1.56.2
+grpcio==1.57.0
     # via opentelemetry-exporter-jaeger-proto-grpc
 httpretty==1.1.4
     # via wipac-rest-tools (setup.py)
@@ -62,7 +62,7 @@ markupsafe==2.1.3
     # via jinja2
 mccabe==0.7.0
     # via flake8
-mypy==1.4.1
+mypy==1.5.1
     # via wipac-rest-tools (setup.py)
 mypy-extensions==1.0.0
     # via mypy
@@ -142,9 +142,9 @@ requests-futures==1.0.1
     # via wipac-rest-tools (setup.py)
 requests-mock==1.11.0
     # via wipac-rest-tools (setup.py)
-ruff==0.0.282
+ruff==0.0.285
     # via wipac-rest-tools (setup.py)
-shellingham==1.5.0.post1
+shellingham==1.5.3
     # via click-completion
 six==1.16.0
     # via
@@ -153,7 +153,7 @@ six==1.16.0
     #   thrift
 thrift==0.16.0
     # via opentelemetry-exporter-jaeger-thrift
-tornado==6.3.2
+tornado==6.3.3
     # via wipac-rest-tools (setup.py)
 types-requests==2.31.0.2
     # via wipac-rest-tools (setup.py)
@@ -168,7 +168,7 @@ typing-extensions==4.7.1
     #   wipac-telemetry
 urllib3==2.0.4
     # via requests
-wheel==0.41.1
+wheel==0.41.2
     # via wipac-rest-tools (setup.py)
 wipac-dev-tools==1.6.16
     # via

dependencies-telemetry.log

@@ -26,7 +26,7 @@ googleapis-common-protos==1.56.2
     # via
     #   opentelemetry-exporter-jaeger-proto-grpc
     #   opentelemetry-exporter-otlp-proto-http
-grpcio==1.56.2
+grpcio==1.57.0
     # via opentelemetry-exporter-jaeger-proto-grpc
 humanfriendly==10.0
     # via coloredlogs
@@ -88,7 +88,7 @@ six==1.16.0
     # via thrift
 thrift==0.16.0
     # via opentelemetry-exporter-jaeger-thrift
-tornado==6.3.2
+tornado==6.3.3
     # via wipac-rest-tools (setup.py)
 typing-extensions==4.7.1
     # via

dependencies-tests.log

@@ -12,15 +12,15 @@ cffi==1.15.1
     # via cryptography
 charset-normalizer==3.2.0
     # via requests
-click==8.1.6
+click==8.1.7
     # via
     #   click-completion
     #   pycycle
 click-completion==0.5.2
     # via pycycle
 colorama==0.4.6
     # via crayons
-coverage[toml]==7.2.7
+coverage[toml]==7.3.0
     # via
     #   pytest-cov
     #   wipac-rest-tools (setup.py)
@@ -83,15 +83,15 @@ requests-futures==1.0.1
     # via wipac-rest-tools (setup.py)
 requests-mock==1.11.0
     # via wipac-rest-tools (setup.py)
-ruff==0.0.282
+ruff==0.0.285
     # via wipac-rest-tools (setup.py)
-shellingham==1.5.0.post1
+shellingham==1.5.3
     # via click-completion
 six==1.16.0
     # via
     #   click-completion
     #   requests-mock
-tornado==6.3.2
+tornado==6.3.3
     # via wipac-rest-tools (setup.py)
 types-requests==2.31.0.2
     # via wipac-rest-tools (setup.py)

dependencies.log

@@ -31,7 +31,7 @@ requests==2.31.0
     #   wipac-rest-tools (setup.py)
 requests-futures==1.0.1
     # via wipac-rest-tools (setup.py)
-tornado==6.3.2
+tornado==6.3.3
     # via wipac-rest-tools (setup.py)
 typing-extensions==4.7.1
     # via

rest_tools/client/client.py

@@ -18,12 +18,13 @@
 
 import jwt
 import requests
+import urllib3
 
 from .. import telemetry as wtt
 from ..utils.json_util import JSONType, json_decode
 from .session import AsyncSession, Session
 
-MAX_RETRIES = 15
+MAX_RETRIES = 30
 
 
 def _to_str(s: Union[str, bytes]) -> str:
@@ -42,6 +43,13 @@ class CalcRetryFromBackoffMax:
 
     backoff_max: float
 
+    def __post_init__(self) -> None:
+        if self.backoff_max > urllib3.util.retry.Retry.DEFAULT_BACKOFF_MAX:
+            raise ValueError(
+                f"CalcRetryFromBackoffMax.backoff_max ({self.backoff_max})"
+                f" cannot be greater than: {urllib3.util.retry.Retry.DEFAULT_BACKOFF_MAX=}"
+            )
+
     def calculate_retries(self, backoff_factor: float) -> int:
         """Calculate the number of retries."""
         #                        backoff_last     =  backoff_factor * 2^(retries-1)
@@ -71,18 +79,26 @@ def calculate_retries(self, timeout: float, backoff_factor: float) -> int:
             )
 
         # the first backoff is always 0 sec, factor applies after 2nd attempt
-        #    T + 0 + sum{n=1,retries-1}(T + 2^n * B)     + T
-        # =  T + 0 + ( T*k + B*2^((retries-1)+1) - 2*B ) + T
-        # not easily solvable (for k) in a closed form
-        for k in range(0, MAX_RETRIES + 2):  # last val -> MAX_RETRIES+1
-            if self.waittime_max > (
+        #    T  +  0  +  sum{n=1,retries-1}(T + min[MAX, 2^n * B] )  +  T
+        # sum has no closed form due to `min` function
+        for candidate in range(0, MAX_RETRIES + 2):  # last val is MAX_RETRIES+1
+            total = (
                 timeout
-                + (timeout * k)
-                + (backoff_factor * 2 ** (k))
-                - (2 * backoff_factor)
+                + 0
+                + sum(
+                    timeout
+                    + min(
+                        urllib3.util.retry.Retry.DEFAULT_BACKOFF_MAX,
+                        2**n * backoff_factor,
+                    )
+                    for n in range(1, candidate)  # 1 to retries-1 (inclusive)
+                )
                 + timeout
-            ):
-                retries = k  # gets overwritten each iteration
+            )
+            # print(candidate)
+            # print(total)
+            if self.waittime_max > total:
+                retries = candidate  # gets overwritten each iteration
             else:
                 break
         return retries
@@ -94,8 +110,14 @@ def _log_retries_values(
     logger.debug(f"using {retries=} {timeout=} {backoff_factor=}")
     if retries:
         retries_schema = ' '.join(
+            # first backoff is 0
             [f'<0.0s> {timeout}s']
-            + [f'<{(backoff_factor * 2**i)}s> {timeout}s' for i in range(1, retries)]
+            +
+            # calculate increasing backoff values
+            [
+                f'<{min(urllib3.util.retry.Retry.DEFAULT_BACKOFF_MAX,backoff_factor * 2**i)}s> {timeout}s'
+                for i in range(1, retries)
+            ]
         )
         logger.debug(
             f"retry scheme (TIMEOUT [<BACKOFF> TIMEOUT ...]): "

tests/unit_client/client_test.py

@@ -11,6 +11,7 @@
 from unittest.mock import Mock
 
 import pytest
+import urllib3
 from httpretty import HTTPretty, httprettified  # type: ignore[import]
 from requests import PreparedRequest
 from requests.exceptions import SSLError, Timeout
@@ -153,9 +154,13 @@ async def test_041_request_autocalc_retries_error() -> None:
     for timeout, backoff_factor, arg in [
         (0.5, 0.75, MAX_RETRIES + 1),
         #
-        (0.5, 0.001, CalcRetryFromBackoffMax(1000)),
+        (
+            0.5,
+            01e-07,
+            CalcRetryFromBackoffMax(urllib3.util.retry.Retry.DEFAULT_BACKOFF_MAX),
+        ),
         #
-        (0.5, 0.001, CalcRetryFromWaittimeMax(1000)),
+        (0.5, 0.3, CalcRetryFromWaittimeMax(60 * 60)),
     ]:
         print(f"{timeout=}, {backoff_factor=}, {arg=}")
         with pytest.raises(
@@ -170,6 +175,24 @@ async def test_041_request_autocalc_retries_error() -> None:
             )
 
 
+@pytest.mark.asyncio
+async def test_042_request_autocalc_retries_error() -> None:
+    """Test auto-calculated retries options in `RestClient`."""
+    with pytest.raises(
+        ValueError,
+        match=r"CalcRetryFromBackoffMax\.backoff_max \(\d+\) cannot be greater than: urllib3\.util\.retry\.Retry\.DEFAULT_BACKOFF_MAX=\d+",
+    ):
+        RestClient(
+            "http://test",
+            "passkey",
+            timeout=0.5,
+            retries=CalcRetryFromBackoffMax(
+                urllib3.util.retry.Retry.DEFAULT_BACKOFF_MAX + 1
+            ),
+            backoff_factor=0.001,
+        )
+
+
 def test_100_request_seq(requests_mock: Mock) -> None:
     """Test `request_seq()`."""
     result = {"result": "the result"}