Set global request timeouts + disable retries (#37)

Author: ThiefMaster

flask_multipass_cern.py

@@ -28,15 +28,31 @@
 CERN_OIDC_WELLKNOWN_URL = 'https://auth.cern.ch/auth/realms/cern/.well-known/openid-configuration'
 # not sure if retries are still needed, but by not using a backoff we don't risk taking down the site
 # using this library in case the API is persistently failing with an error
-HTTP_RETRY_COUNT = 2
-retry_config = HTTPAdapter(max_retries=Retry(total=HTTP_RETRY_COUNT,
-                                             backoff_factor=0,
-                                             status_forcelist=[503, 504],
-                                             allowed_methods=frozenset(['GET']),
-                                             raise_on_status=False))
+HTTP_RETRY_COUNT = 0
+retry_config = Retry(
+    total=HTTP_RETRY_COUNT,
+    backoff_factor=0,
+    status_forcelist=[503, 504],
+    allowed_methods=frozenset(['GET']),
+    raise_on_status=False,
+)
 _cache_miss = object()
 
 
+class TimeoutHTTPAdapter(HTTPAdapter):
+    def __init__(self, *args, timeout=None, **kwargs):
+        self._default_timeout = timeout
+        super().__init__(*args, **kwargs)
+
+    def send(self, request, *, timeout=None, **kwargs):
+        if timeout is None:
+            timeout = self._default_timeout
+        return super().send(request, timeout=timeout, **kwargs)
+
+
+http_adapter = TimeoutHTTPAdapter(timeout=10, max_retries=retry_config)
+
+
 class ExtendedCache:
     def __init__(self, cache):
         self.cache = self._init_cache(cache)
@@ -393,7 +409,7 @@ def search_identities_ex(self, criteria, exact=False, limit=None):
             results = []
             total = 0
             try:
-                results, total = self._fetch_all(api_session, '/api/v1.0/Identity', params, limit=limit)
+                results, total = self._fetch_all(api_session, '/api/v1.0/Identity', params, limit=limit, timeout=20)
             except RequestException:
                 self.logger.warning('Refreshing identities failed for criteria %s', criteria)
                 if use_cache and cached_data:
@@ -483,7 +499,7 @@ def _get_api_session(self):
         token = self.cache.get(cache_key)
         if token:
             oauth_session = OAuth2Session(token=token, leeway=0)
-            oauth_session.mount(self.authz_api_base, retry_config)
+            oauth_session.mount(self.authz_api_base, http_adapter)
             return oauth_session
         meta = self.authlib_client.load_server_metadata()
         token_endpoint = meta['token_endpoint'].replace('protocol/openid-connect', 'api-access')
@@ -494,7 +510,7 @@ def _get_api_session(self):
             grant_type='client_credentials',
             leeway=0,  # we handle leeway ourselves via the cache duration
         )
-        oauth_session.mount(self.authz_api_base, retry_config)
+        oauth_session.mount(self.authz_api_base, http_adapter)
         oauth_session.fetch_access_token(
             audience='authorization-service-api',
             headers={'Content-Type': 'application/x-www-form-urlencoded;charset=UTF-8'},
@@ -522,9 +538,9 @@ def _fetch_identity_data(self, auth_info):
         del data['id']  # id is always included
         return data
 
-    def _fetch_all(self, api_session, endpoint, params, limit=None, *, empty_if_404=False):
+    def _fetch_all(self, api_session, endpoint, params, limit=None, *, empty_if_404=False, timeout=None):
         results = []
-        resp = api_session.get(self.authz_api_base + endpoint, params=params)
+        resp = api_session.get(self.authz_api_base + endpoint, params=params, timeout=timeout)
         if empty_if_404 and resp.status_code == 404:
             return [], 0
         resp.raise_for_status()
@@ -535,7 +551,7 @@ def _fetch_all(self, api_session, endpoint, params, limit=None, *, empty_if_404=
             results += data['data']
             if not data['pagination']['next'] or (limit is not None and len(results) >= limit):
                 break
-            resp = api_session.get(self.authz_api_base + data['pagination']['next'])
+            resp = api_session.get(self.authz_api_base + data['pagination']['next'], timeout=timeout)
             # XXX we do not expect 404s here after the first one succeeded, so here we consider it a real error
             resp.raise_for_status()
             data = resp.json()

tests/conftest.py

@@ -7,7 +7,7 @@
 from flask_multipass import Multipass
 from requests.sessions import Session
 
-from flask_multipass_cern import CERNIdentityProvider, retry_config
+from flask_multipass_cern import CERNIdentityProvider, http_adapter
 
 
 class MemoryCacheEntry:
@@ -49,7 +49,7 @@ def httpretty_enabled():
 def mock_get_api_session(mocker):
     mock_session = mocker.patch('flask_multipass_cern.CERNIdentityProvider._get_api_session')
     mock_session.return_value = Session()
-    mock_session.return_value.mount('https://authorization-service-api.web.cern.ch', retry_config)
+    mock_session.return_value.mount('https://authorization-service-api.web.cern.ch', http_adapter)
     return mock_session
 
 

tests/test_request_retry.py

@@ -7,7 +7,7 @@
 
 @pytest.fixture(autouse=True)
 def faster_retries(monkeypatch):
-    monkeypatch.setattr('flask_multipass_cern.retry_config.max_retries.backoff_factor', 0)
+    monkeypatch.setattr('flask_multipass_cern.http_adapter.max_retries.backoff_factor', 0)
 
 
 @pytest.mark.usefixtures('httpretty_enabled', 'mock_get_api_session')