Skip to content

Commit

Permalink
Set global request timeouts + disable retries
Browse files Browse the repository at this point in the history
  • Loading branch information
ThiefMaster committed Jan 16, 2025
1 parent 6d9b379 commit 41b7897
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 15 deletions.
40 changes: 28 additions & 12 deletions flask_multipass_cern.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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')
Expand All @@ -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'},
Expand Down Expand Up @@ -529,9 +545,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()
Expand All @@ -542,7 +558,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()
Expand Down
4 changes: 2 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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


Expand Down
2 changes: 1 addition & 1 deletion tests/test_request_retry.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down

0 comments on commit 41b7897

Please sign in to comment.