♻️ chore: refactor retry logic for API calls

osc_sdk_python/__init__.py

@@ -6,6 +6,7 @@
 from .version import get_version
 from .problem import Problem, ProblemDecoder
 from .limiter import RateLimiter
+from .retry import Retry
 
 # what to Log
 from .outscale_gateway import LOG_ALL
@@ -26,4 +27,5 @@
     "Problem",
     "ProblemDecoder",
     "RateLimiter",
+    "Retry",
 ]

osc_sdk_python/call.py

@@ -3,20 +3,13 @@
 from .credentials import Profile
 from .requester import Requester
 from requests import Session
-from requests.adapters import HTTPAdapter
-from urllib3.util.retry import Retry
 from urllib3.util import parse_url
 from datetime import timedelta
 from .limiter import RateLimiter
 
 import json
 import warnings
 
-MAX_RETRIES = 3
-RETRY_BACKOFF_FACTOR = "1"
-RETRY_BACKOFF_JITTER = "3"
-RETRY_BACKOFF_MAX = "30"
-
 
 class Call(object):
     def __init__(self, logger=None, limiter=None, **kwargs):
@@ -26,15 +19,12 @@ def __init__(self, logger=None, limiter=None, **kwargs):
         self.user_agent = kwargs.pop("user_agent", DEFAULT_USER_AGENT)
         self.logger = logger
         self.limiter: RateLimiter | None = limiter
-        self.adapter = None
+        self.retry_kwargs = {}
         self.session = Session()
 
         kwargs = self.update_limiter(**kwargs)
-        kwargs = self.update_adapter(**kwargs)
+        kwargs = self.update_retry(**kwargs)
         self.update_profile(**kwargs)
-        if self.adapter:
-            self.session.mount("https://", self.adapter)
-            self.session.mount("http://", self.adapter)
 
     def update_credentials(self, **kwargs):
         warnings.warn(
@@ -44,32 +34,6 @@ def update_credentials(self, **kwargs):
         )
         return self.update_profile(**kwargs)
 
-    def update_adapter(self, **kwargs):
-        max_retries: int | str | None = kwargs.pop("max_retries", None)
-        if max_retries is not None:
-            max_retries = int(max_retries)
-        else:
-            max_retries = MAX_RETRIES
-
-        if max_retries > 0:
-            self.adapter = HTTPAdapter(
-                max_retries=Retry(
-                    total=max_retries,
-                    backoff_factor=float(
-                        kwargs.pop("retry_backoff_factor", RETRY_BACKOFF_FACTOR)
-                    ),
-                    backoff_jitter=float(
-                        kwargs.pop("retry_backoff_jitter", RETRY_BACKOFF_JITTER)
-                    ),
-                    backoff_max=float(
-                        kwargs.pop("retry_backoff_max", RETRY_BACKOFF_MAX)
-                    ),
-                    status_forcelist=(400, 429, 500, 503),
-                    allowed_methods=("POST", "GET"),
-                )
-            )
-        return kwargs
-
     def update_profile(self, **kwargs):
         self.profile = Profile.from_standard_configuration(
             kwargs.pop("path", None), kwargs.pop("profile", None)
@@ -88,6 +52,17 @@ def update_limiter(self, **kwargs):
 
         return kwargs
 
+    def update_retry(self, **kwargs):
+        max_retries = kwargs.pop("max_retries", None)
+        if max_retries is not None:
+            self.retry_kwargs["max_retries"] = int(max_retries)
+
+        for key in ["backoff_factor", "backoff_jitter", "backoff_max"]:
+            value = kwargs.pop(f"retry_{key}", None)
+            if value is not None:
+                self.retry_kwargs[key] = float(value)
+        return kwargs
+
     def api(self, action, service="api", **data):
         try:
             endpoint = self.profile.get_endpoint(service) + "/" + action
@@ -106,6 +81,7 @@ def api(self, action, service="api", **data):
                     user_agent=self.user_agent,
                 ),
                 endpoint,
+                **self.retry_kwargs,
             )
             if self.logger is not None:
                 self.logger.do_log(

osc_sdk_python/requester.py

@@ -1,18 +1,12 @@
-from requests import HTTPError
-from requests.exceptions import JSONDecodeError
-from .problem import ProblemDecoder, LegacyProblemDecoder, LegacyProblem, Problem
+from .retry import Retry
 
 
 class Requester:
-    def __init__(
-        self,
-        session,
-        auth,
-        endpoint,
-    ):
+    def __init__(self, session, auth, endpoint, **kwargs):
         self.session = session
         self.auth = auth
         self.endpoint = endpoint
+        self.request_kwargs = kwargs
 
     def send(self, uri, payload):
         headers = None
@@ -26,57 +20,10 @@ def send(self, uri, payload):
         else:
             cert_file = None
 
-        response = self.session.post(
-            self.endpoint,
-            data=payload,
-            headers=headers,
-            verify=True,
-            cert=cert_file,
+        retry_kwargs = self.request_kwargs.copy()
+        retry_kwargs.update(
+            {"data": payload, "headers": headers, "verify": True, "cert": cert_file}
         )
-        self.raise_for_status(response)
-        return response.json()
 
-    def raise_for_status(self, response):
-        http_error_msg = ""
-        problem = None
-        reason = self.get_default_reason(response)
-
-        try:
-            ct = response.headers.get("content-type") or ""
-            if "application/json" in ct:
-                problem = response.json(cls=LegacyProblemDecoder)
-                problem.status = problem.status or str(response.status_code)
-                problem.url = response.url
-            elif "application/problem+json" in ct:
-                problem = response.json(cls=ProblemDecoder)
-                problem.status = problem.status or str(response.status_code)
-        except JSONDecodeError:
-            pass
-        else:
-            if 400 <= response.status_code < 500:
-                if isinstance(problem, LegacyProblem) or isinstance(problem, Problem):
-                    http_error_msg = f"Client Error --> {problem.msg()}"
-                else:
-                    http_error_msg = f"{response.status_code} Client Error: {reason} for url: {response.url}"
-
-            elif 500 <= response.status_code < 600:
-                if isinstance(problem, LegacyProblem) or isinstance(problem, Problem):
-                    http_error_msg = f"Server Error --> {problem.msg()}"
-                else:
-                    http_error_msg = f"{response.status_code} Server Error: {reason} for url: {response.url}"
-
-            if http_error_msg:
-                raise HTTPError(http_error_msg, response=response)
-
-    def get_default_reason(self, response):
-        if isinstance(response.reason, bytes):
-            # We attempt to decode utf-8 first because some servers
-            # choose to localize their reason strings. If the string
-            # isn't utf-8, we fall back to iso-8859-1 for all other
-            # encodings. (See PR #3538)
-            try:
-                return response.reason.decode("utf-8")
-            except UnicodeDecodeError:
-                return response.reason.decode("iso-8859-1")
-        else:
-            return response.reason
+        response = Retry(self.session, "post", self.endpoint, **retry_kwargs)
+        return response.execute().json()

osc_sdk_python/retry.py

@@ -0,0 +1,138 @@
+import requests
+import time
+import random
+from requests.exceptions import JSONDecodeError
+from .problem import ProblemDecoder, LegacyProblemDecoder, LegacyProblem, Problem
+
+MAX_RETRIES = 3
+RETRY_BACKOFF_FACTOR = 1.0
+RETRY_BACKOFF_JITTER = 3.0
+RETRY_BACKOFF_MAX = 30.0
+
+
+class Retry:
+    """
+    Hold a request attempt and try to execute it
+    """
+
+    def __init__(self, session: requests.Session, method: str, url: str, **kwargs):
+        self.session = session
+        self.method: str = method
+        self.url: str = url
+        self.request_kwargs = kwargs
+
+        # Extract all retry parameters
+        self.attempt: int = int(self.request_kwargs.pop("attempt", 0))
+        self.max_retries: int = int(
+            self.request_kwargs.get("max_retries", MAX_RETRIES)
+        )
+        self.backoff_factor: float = float(
+            self.request_kwargs.get("backoff_factor", RETRY_BACKOFF_FACTOR)
+        )
+        self.backoff_jitter: float = float(
+            self.request_kwargs.get("backoff_jitter", RETRY_BACKOFF_JITTER)
+        )
+        self.backoff_max: float = float(
+            self.request_kwargs.get("backoff_max", RETRY_BACKOFF_MAX)
+        )
+
+    def execute_once(self) -> requests.Response:
+        """
+        Execute the request without retry
+        """
+        return self.session.request(self.method, self.url, **self.request_kwargs)
+
+    def increment(self) -> "Retry":
+        """
+        Return a copy of the retry with an incremented attempt count
+        """
+        new_kwargs = self.request_kwargs.copy()
+        new_kwargs["attempt"] = self.attempt + 1
+        return Retry(self.session, self.method, self.url, **new_kwargs)
+
+    def should_retry(self, e: requests.exceptions.RequestException) -> bool:
+        if isinstance(e, requests.exceptions.TooManyRedirects):
+            return False
+
+        if isinstance(e, requests.exceptions.URLRequired):
+            return False
+
+        if isinstance(e, ValueError):
+            # can be raised on bogus request
+            return False
+
+        if e.response is not None:
+            if 400 <= e.response.status_code < 500 and e.response.status_code != 429:
+                return False
+
+        return self.attempt < self.max_retries
+
+    def get_backoff_time(self) -> float:
+        """
+        {backoff factor} * (2 ** ({number of previous retries}))
+        random.uniform(0, {backoff jitter})
+        """
+
+        backoff: float = self.backoff_factor * (2**self.attempt)
+        backoff += random.uniform(0, self.backoff_jitter)
+        return min(backoff, self.backoff_max)
+
+    def execute(self) -> requests.Response:
+        try:
+            res = self.execute_once()
+            raise_for_status(res)
+            return res
+        except requests.exceptions.RequestException as e:
+            if self.should_retry(e):
+                sleep_time = self.get_backoff_time()
+                time.sleep(sleep_time)
+                return self.increment().execute()
+            else:
+                raise e
+
+
+def raise_for_status(response: requests.Response):
+    http_error_msg = ""
+    problem = None
+    reason = get_default_reason(response)
+
+    try:
+        ct = response.headers.get("content-type") or ""
+        if "application/json" in ct:
+            problem = response.json(cls=LegacyProblemDecoder)
+            problem.status = problem.status or str(response.status_code)
+            problem.url = response.url
+        elif "application/problem+json" in ct:
+            problem = response.json(cls=ProblemDecoder)
+            problem.status = problem.status or str(response.status_code)
+    except JSONDecodeError:
+        pass
+    else:
+        if 400 <= response.status_code < 500:
+            if isinstance(problem, LegacyProblem) or isinstance(problem, Problem):
+                http_error_msg = f"Client Error --> {problem.msg()}"
+            else:
+                http_error_msg = f"{response.status_code} Client Error: {reason} for url: {response.url}"
+
+        elif 500 <= response.status_code < 600:
+            if isinstance(problem, LegacyProblem) or isinstance(problem, Problem):
+                http_error_msg = f"Server Error --> {problem.msg()}"
+            else:
+                http_error_msg = f"{response.status_code} Server Error: {reason} for url: {response.url}"
+
+        if http_error_msg:
+            raise requests.HTTPError(http_error_msg, response=response)
+
+
+def get_default_reason(response):
+    if isinstance(response.reason, bytes):
+        # We attempt to decode utf-8 first because some servers
+        # choose to localize their reason strings. If the string
+        # isn't utf-8, we fall back to iso-8859-1 for all other
+        # encodings. (See PR #3538)
+        try:
+            return response.reason.decode("utf-8")
+        except UnicodeDecodeError:
+            return response.reason.decode("iso-8859-1")
+    else:
+        return response.reason

tests/test_exceptions.py

@@ -3,15 +3,14 @@
 
 sys.path.append("..")
 from osc_sdk_python import Gateway
-from requests.exceptions import RetryError
+from requests.exceptions import HTTPError
 
 
 class TestExcept(unittest.TestCase):
-
     def test_listing(self):
         gw = Gateway()
         # a is not a valide argument
-        with self.assertRaises(RetryError):
+        with self.assertRaises(HTTPError):
             gw.ReadVms(Filters="a")
 
 

tests/test_log.py

@@ -6,7 +6,6 @@
 
 
 class TestLog(unittest.TestCase):
-
     def test_listing(self):
         gw = Gateway()
         gw.log.config(type=LOG_MEMORY, what=LOG_KEEP_ONLY_LAST_REQ)