Introduce Http retry strategy (#16)

lockable/provider.py

@@ -4,7 +4,6 @@
 import typing
 from typing import List
 
-from urllib.parse import urlparse
 from pydash import filter_, count_by
 
 from lockable.logger import get_logger
@@ -29,15 +28,6 @@ def data(self) -> list:
         """ Get resources list """
         return self._resources
 
-    @staticmethod
-    def is_http_url(uri: str) -> bool:
-        """ Check if argument is url format"""
-        try:
-            result = urlparse(uri)
-            return all([result.scheme, result.netloc])
-        except:  # pylint: disable=bare-except
-            return False
-
     @abstractmethod
     def reload(self) -> None:  # pragma: no cover
         """ Reload resources data"""

lockable/provider_helpers.py

@@ -1,5 +1,5 @@
 """ resources Provider helper """
-from lockable.provider import Provider
+from urllib.parse import urlparse
 from lockable.provider_list import ProviderList
 from lockable.provider_file import ProviderFile
 from lockable.provider_http import ProviderHttp
@@ -12,10 +12,19 @@ def create(uri):
     :return: Provider object
     :rtype: Provider
     """
-    if Provider.is_http_url(uri):
+    if is_http_url(uri):
         return ProviderHttp(uri)
     if isinstance(uri, str):
         return ProviderFile(uri)
     if isinstance(uri, list):
         return ProviderList(uri)
     raise AssertionError('uri should be list or string')
+
+
+def is_http_url(uri: str) -> bool:
+    """ Check if argument is url format"""
+    try:
+        result = urlparse(uri)
+        return all([result.scheme, result.netloc])
+    except:  # pylint: disable=bare-except
+        return False

lockable/provider_http.py

@@ -1,40 +1,94 @@
 """ resources Provider for HTTP """
 import requests
-from requests import HTTPError
+from requests import HTTPError, ConnectionError as RequestConnectionError
+from requests.adapters import HTTPAdapter
+from urllib3.util.retry import Retry
+from urllib3.util import parse_url
+from urllib3.exceptions import MaxRetryError
 
 from lockable.provider import Provider, ProviderError
 from lockable.logger import get_logger
 
 MODULE_LOGGER = get_logger()
 
 
+class RetryWithLogging(Retry):
+    """ urllib3.util.retry Retry overwrite to add logging """
+    def increment(self, *args, **kwargs):
+        try:
+            error = kwargs['error']
+            MODULE_LOGGER.warning('retried http resources GET due to %s', error)
+        except KeyError:
+            pass
+
+        return super().increment(*args, **kwargs)
+
+
 class ProviderHttp(Provider):
     """ ProviderHttp interface"""
 
+    TOTAL_RETRIES = 9  # This should be enough even we update server with short service break
+    REDIRECT = 5  # redirect max count
+    BACKOFF_FACTOR = 1  # [0.0s, 1s, 2s, 4s, 8s, 16s, 32s, 1min4s, 2min8s]
+
     def __init__(self, uri: str):
         """ ProviderHttp constructor """
+        self._configure_http_strategy(uri)
         super().__init__(uri)
 
+    def _configure_http_strategy(self, uri):
+        """ configure http Strategy """
+        retry_strategy = RetryWithLogging(
+            total=ProviderHttp.TOTAL_RETRIES,
+            redirect=ProviderHttp.REDIRECT,
+            connect=ProviderHttp.TOTAL_RETRIES,
+            other=ProviderHttp.TOTAL_RETRIES,
+            raise_on_status=False,
+            status_forcelist=[
+                429,  # Too Many Requests
+                500,  # Internal Server Error
+                502,  # Bad Gateway server error
+                503,  # Service Unavailable
+                504  # Gateway Timeout server error
+            ],
+            backoff_factor=ProviderHttp.BACKOFF_FACTOR
+        )
+
+        #  create http adapter with retry strategy
+        adapter = HTTPAdapter(max_retries=retry_strategy)
+        self._http = requests.Session()
+
+        url = parse_url(uri)
+        self._http.mount(f'{url.scheme}://', adapter)
+
     def reload(self) -> None:
         """ Reload resources list from web server """
-        self.set_resources_list(self._get_http(self._uri))
+        self.set_resources_list(self._get_list())
 
-    @staticmethod
-    def _get_http(uri: str) -> list:
+    def _get_list(self) -> list:
         """ Internal method to get http json data"""
         try:
-            response = requests.get(uri)
-            response.raise_for_status()
+            response = self._http.get(self._uri)
 
             # could utilise ETag or Last-Modified headers to optimize performance
             # etag = response.headers.get("ETag")
             # last_modified = response.headers.get("Last-Modified")
 
+            # if we get non retry_strategy based response we still
+            # have to check if response is success, e.g. not 404..
+            response.raise_for_status()
+
             # access JSON content
             return response.json()
         except HTTPError as http_err:
             MODULE_LOGGER.error('HTTP error occurred %s', http_err)
             raise ProviderError(http_err.response.reason) from http_err
-        except Exception as err:
-            MODULE_LOGGER.error('Other error occurred: %s', err)
-            raise ProviderError(err) from err
+        except RequestConnectionError as error:
+            MODULE_LOGGER.error('Connection error: %s', error)
+            raise ProviderError(error) from error
+        except MaxRetryError as error:
+            MODULE_LOGGER.error('Max retries error: %s', error)
+            raise ProviderError(error) from error
+        except Exception as error:
+            MODULE_LOGGER.error('Other error occurred: %s', error)
+            raise ProviderError(error) from error

requirements.txt

@@ -9,3 +9,5 @@ pydash==5.0.0
 requests==2.26.0
 setuptools-scm==6.0.1
 urllib3==1.26.6
+lockable~=0.3.2.dev5+g2ca5346
+setuptools~=56.0.0

tests/test_Provider.py

@@ -3,6 +3,8 @@
 import os
 from tempfile import TemporaryDirectory
 from unittest import TestCase
+from unittest.mock import MagicMock
+
 from lockable.provider import Provider, ProviderError
 from lockable.provider_list import ProviderList
 from lockable.provider_http import ProviderHttp
@@ -24,6 +26,23 @@ def do_GET(self):
         self.wfile.write(contents)
 
 
+class TestHTTPServer429(httptest.Handler):
+
+    statuses = [429, 429, 200]
+    call = 0
+
+    def do_GET(self):
+        contents = "[{\"id\": \"abc\"}]".encode()
+        self.send_response(self.statuses[TestHTTPServer429.call % len(self.statuses)])
+        TestHTTPServer429.call += 1
+        self.send_header("ETag", "1234567890")
+        self.send_header("Last-Modified", "Mon, 01 Jan 1970 00:00:00 GMT")
+        self.send_header("Content-type", "text/json")
+        self.send_header("Content-length", len(contents))
+        self.end_headers()
+        self.wfile.write(contents)
+
+
 class TestHTTPServer404(httptest.Handler):
 
     def do_GET(self):
@@ -36,6 +55,7 @@ def do_GET(self):
         self.end_headers()
         self.wfile.write(contents)
 
+
 class TestHTTPServerInvalidData(httptest.Handler):
 
     def do_GET(self):
@@ -46,11 +66,20 @@ def do_GET(self):
         self.end_headers()
         self.wfile.write(contents)
 
+
 class ProviderTests(TestCase):
     def setUp(self) -> None:
         logger = logging.getLogger('lockable')
         logger.handlers.clear()
         logger.addHandler(logging.NullHandler())
+        # backup
+        self._total_retries = ProviderHttp.TOTAL_RETRIES
+        self._backoff_factor = ProviderHttp.BACKOFF_FACTOR
+
+    def tearDown(self) -> None:
+        # restore
+        ProviderHttp.TOTAL_RETRIES = self._total_retries
+        ProviderHttp.BACKOFF_FACTOR = self._backoff_factor
 
     def test_create_raises(self):
         with self.assertRaises(FileNotFoundError):
@@ -114,3 +143,23 @@ def test_provider_http_invalid_data(self, ts=httptest.NoServer()):
         ts.server_name = 'localhost'
         with self.assertRaises(ProviderError):
             create_provider(ts.url())
+
+    @httptest.Server(TestHTTPServer429)
+    def test_provider_http_too_many_requests_eventually_success(self, ts=httptest.NoServer()):
+        ts.server_name = 'localhost'
+        create_provider(ts.url())
+
+    @httptest.Server(TestHTTPServer429)
+    def test_provider_http_too_many_requests_fails(self, ts=httptest.NoServer()):
+        ts.server_name = 'localhost'
+        create_provider(ts.url())
+        ProviderHttp.TOTAL_RETRIES = 2
+        ProviderHttp.BACKOFF_FACTOR = 0
+        with self.assertRaises(ProviderError):
+            create_provider('http://localhost/resource')
+
+    def test_provider_http_no_response(self):
+        ProviderHttp.TOTAL_RETRIES = 2
+        ProviderHttp.BACKOFF_FACTOR = 0
+        with self.assertRaises(ProviderError):
+            create_provider('http://localhost/resource')