diff --git a/maap/config_reader.py b/maap/config_reader.py index e2f82e3..2c2bf73 100644 --- a/maap/config_reader.py +++ b/maap/config_reader.py @@ -33,7 +33,8 @@ import requests from urllib.parse import urlparse, urljoin, urlunsplit, SplitResult from collections import namedtuple -from functools import cache +from requests.adapters import HTTPAdapter +from urllib3.util.retry import Retry logger = logging.getLogger(__name__) @@ -115,10 +116,25 @@ def _get_api_root(config_url, config): query='', fragment='').geturl() -@cache +_CONFIG_CACHE = {} +_CONFIG_TIMEOUT = (5, 30) # (connect, read) seconds +_CONFIG_RETRY = Retry( + total=5, + connect=5, + read=3, + status=5, + backoff_factor=1, + status_forcelist=(502, 503, 504), + allowed_methods=frozenset(["GET"]), +) + + def _get_client_config(maap_host): """ - Fetch and cache client configuration from the API. + Fetch client configuration from the API, caching successful results. + + Requests use a timeout and retry with exponential backoff on connection + errors and 502/503/504 responses. Failures are never cached. Parameters ---------- @@ -130,21 +146,28 @@ def _get_client_config(maap_host): dict Configuration dictionary containing service endpoints and settings. - Notes - ----- - Results are cached using functools.cache to avoid repeated API calls - for the same host. + Raises + ------ + RuntimeError + If the configuration cannot be fetched or parsed. """ + if maap_host in _CONFIG_CACHE: + return _CONFIG_CACHE[maap_host] config_url = _get_config_url(maap_host) logger.debug(f"Requesting client config from api at: {config_url}") - response = requests.get(config_url) try: - response.raise_for_status() - config = response.json() + with requests.Session() as session: + session.mount("http://", HTTPAdapter(max_retries=_CONFIG_RETRY)) + session.mount("https://", HTTPAdapter(max_retries=_CONFIG_RETRY)) + response = session.get(config_url, timeout=_CONFIG_TIMEOUT) + response.raise_for_status() + config = response.json() config["service"]["maap_api_root"] = _get_api_root(config_url, config) - return config except Exception as ex: logger.error(f"Unable to read maap config from api: {ex}") + raise RuntimeError(f"Unable to read maap config from {config_url}: {ex}") from ex + _CONFIG_CACHE[maap_host] = config + return config class MaapConfig: diff --git a/test/test_config_reader.py b/test/test_config_reader.py index f219edf..36e4f45 100644 --- a/test/test_config_reader.py +++ b/test/test_config_reader.py @@ -77,3 +77,48 @@ def test_read_conf_default(self): def test_read_conf_uninitialized(self): with TestConfig().assertRaises(expected_exception=EnvironmentError, msg="Successfully raised exception") as err: MaapConfig() + + +class TestClientConfigFetch(TestCase): + CONFIG = {"service": {"maap_api_root": "https://api.example.org/api"}} + + def setUp(self): + config_reader._CONFIG_CACHE.clear() + + def _resp(self, status=200): + from unittest import mock + resp = mock.Mock(status_code=status) + resp.json.return_value = {"service": dict(self.CONFIG["service"])} + if status >= 400: + import requests + resp.raise_for_status.side_effect = requests.HTTPError(str(status)) + return resp + + def test_transient_failure_then_success(self): + from unittest import mock + import requests + with mock.patch("requests.Session.get", + side_effect=[requests.ConnectionError("boom"), self._resp()]) as get: + with self.assertRaises(RuntimeError): + config_reader._get_client_config("https://api.example.org") + config = config_reader._get_client_config("https://api.example.org") + self.assertEqual(2, get.call_count) + self.assertEqual("https://api.example.org/api/", config["service"]["maap_api_root"]) + + def test_permanent_failure_not_cached(self): + from unittest import mock + with mock.patch("requests.Session.get", return_value=self._resp(503)) as get: + for _ in range(2): + with self.assertRaises(RuntimeError): + config_reader._get_client_config("https://api.example.org") + self.assertEqual(2, get.call_count) + self.assertNotIn("https://api.example.org", config_reader._CONFIG_CACHE) + + def test_success_is_cached_and_timeout_used(self): + from unittest import mock + with mock.patch("requests.Session.get", return_value=self._resp()) as get: + first = config_reader._get_client_config("https://api.example.org") + second = config_reader._get_client_config("https://api.example.org") + self.assertIs(first, second) + self.assertEqual(1, get.call_count) + self.assertEqual(config_reader._CONFIG_TIMEOUT, get.call_args.kwargs["timeout"])