Merge pull request #86 from appsembler/default_timeout

enforce a default timeout if SITE_CONFIG_CACHE_TIMEOUT is not set

CHANGELOG.md

@@ -7,7 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 <!-- Note: Update the `Unreleased link` after adding a new release -->
 
-## [Unreleased](https://github.com/appsembler/tahoe-sites/compare/v0.2.3...HEAD)
+## [Unreleased](https://github.com/appsembler/tahoe-sites/compare/v0.2.4...HEAD)
+
+## [0.2.4](https://github.com/appsembler/site-configuration-client/compare/v0.2.3...v0.2.4) - 2022-09-27
+ - enforce a default 300sec timeout if `SITE_CONFIG_CACHE_TIMEOUT` is not set
+
 
 ## [0.2.3](https://github.com/appsembler/site-configuration-client/compare/v0.2.2...v0.2.3) - 2022-09-16
  - Get `openedx.api` values for a specific Open edX site.

setup.py

@@ -8,7 +8,7 @@ def read(fname):
 
 setup(
     name='site-configuration-client',
-    version='0.2.3',
+    version='0.2.4',
     description='Python client library for Site Configuration API',
     long_description=read('README.rst'),
     classifiers=[

site_config_client/django_cache.py

@@ -7,9 +7,12 @@ class DjangoCache:
     """
     Allows Site Configuration client to configure caching with Django
     """
-    def __init__(self, cache_name, cache_timeout=300):
+
+    DEFAULT_TIMEOUT = 300  # 5 minutes
+
+    def __init__(self, cache_name, cache_timeout):
         self.cache_name = cache_name
-        self.cache_timeout = cache_timeout
+        self.cache_timeout = int(cache_timeout or self.DEFAULT_TIMEOUT)
         self._django_cache = None
 
     def get_django_cache(self):
@@ -22,10 +25,7 @@ def get_django_cache(self):
         return self._django_cache
 
     def set(self, key, value):
-        kwargs = {}
-        if self.cache_timeout is not None:
-            kwargs['timeout'] = self.cache_timeout
-        return self.get_django_cache().set(key, value, **kwargs)
+        return self.get_django_cache().set(key, value, timeout=self.cache_timeout)
 
     def get(self, key):
         return self.get_django_cache().get(key)

tests/test_client.py

@@ -249,7 +249,7 @@ def test_get_backend_configs_error(requests_mock, site_config_client):
 
 @pytest.mark.django
 def test_get_backend_configs_cache(requests_mock, site_config_client):
-    site_config_client.cache = DjangoCache(cache_name='default')
+    site_config_client.cache = DjangoCache(cache_name='default', cache_timeout=300)
     backend_live_configs_path = (
         'http://service/v1/environment/staging/combined-configuration/backend/{}/live/'
         .format(PARAMS['uuid']))

tests/test_django_cache.py

@@ -14,7 +14,7 @@ def django_cache_adapter(settings):
             'BACKEND': 'django.core.cache.backends.locmem.LocMemCache',
         }
     }
-    return DjangoCache(cache_name='default')
+    return DjangoCache(cache_name='default', cache_timeout=100)
 
 
 @pytest.mark.django
@@ -23,6 +23,22 @@ def test_empty_get(django_cache_adapter):
     assert django_cache_adapter.get(key=cache_key) is None
 
 
+@pytest.mark.django
+@pytest.mark.parametrize('params,expected_timeout,message', [
+    ({'cache_timeout': None}, 300, 'None defaults to 300'),
+    ({'cache_timeout': ''}, 300, 'Empty string --> 300'),
+    ({'cache_timeout': '600'}, 600, 'string is converted to int'),
+    ({'cache_timeout': 800}, 800, 'int 800 is kept as-is'),
+])
+def test_timeout(params, expected_timeout, message):
+    """
+    Ensure `cache_timeout` is can be configured with both int and str with sane defaults.
+    """
+    from site_config_client.django_cache import DjangoCache
+    cache = DjangoCache(cache_name='default', **params)
+    assert cache.cache_timeout == expected_timeout, message
+
+
 @pytest.mark.django
 def test_set_get(django_cache_adapter):
     configs = {

tests/test_plugin_settings.py

@@ -60,6 +60,7 @@ def test_plugin_production_settings_no_client(caplog):
     settings = mock.Mock(
         SITE_CONFIG_BASE_URL=None,
         SITE_CONFIG_READ_ONLY_BUCKET=None,
+        SITE_CONFIG_CACHE_TIMEOUT=None,
     )
     plugin_settings(settings)
     assert settings.SITE_CONFIG_CLIENT, 'Client should not be initialized due to missing SITE_CONFIG_BASE_URL'
@@ -74,6 +75,7 @@ def test_plugin_test_settings(client):
 
     settings = mock.Mock(
         SITE_CONFIG_CLIENT=None,
+        SITE_CONFIG_CACHE_TIMEOUT='',
     )
 
     plugin_settings(settings)