Configuration logic extracted to simple class. Now it is common for d…

…ata sources and cache classes.

src/core/caches/base.py

@@ -2,12 +2,14 @@
 import logging
 import pickle
 
+from ..common.config import LongitudeConfigurable
 
-class LongitudeCache:
-    default_config = {}
+
+class LongitudeCache(LongitudeConfigurable):
+    _default_config = {}
 
     def __init__(self, config=None):
-        self._config = config or self.default_config
+        super().__init__(config=config)
         self.logger = logging.getLogger(self.__class__.__module__)
 
     @staticmethod

src/core/caches/redis.py

@@ -2,25 +2,22 @@
 from .base import LongitudeCache
 
 
-class RedisCacheConfig:
-    def __init__(self, host='localhost', port=6379, db=0, password=None):
-        self.host = host
-        self.port = port
-        self.db = db
-        self.password = password
-
-
 class RedisCache(LongitudeCache):
-    _default_config = RedisCacheConfig()
+    _default_config = {
+        'host': 'localhost',
+        'port': 6379,
+        'db': 0,
+        'password': None
+    }
 
     _values = None
 
     def setup(self):
         self._values = redis.Redis(
-            host=self._config.host,
-            port=self._config.port,
-            db=self._config.db,
-            password=self._config.password
+            host=self.get_config('host'),
+            port=self.get_config('port'),
+            db=self.get_config('db'),
+            password=self.get_config('password')
         )
 
     @property
@@ -31,7 +28,8 @@ def is_ready(self):
         except TimeoutError:
             return False
         except redis.exceptions.ConnectionError:
-            self.logger.error('Cannot connect to Redis server at %s:%d.' % (self._config.host, self._config.port))
+            self.logger.error(
+                'Cannot connect to Redis server at %s:%d.' % (self.get_config('host'), self.get_config('port')))
             return False
         except redis.exceptions.ResponseError as e:
             msg = str(e)

src/core/common/__init__.py

@@ -0,0 +1 @@
+

src/core/common/config.py

@@ -0,0 +1,48 @@
+import logging
+
+from .exceptions import LongitudeConfigError
+
+
+class LongitudeConfigurable:
+    """
+    Any subclass will have a nice get_config(key) method to retrieve configuration values
+    """
+    _default_config = {}
+    _config = {}
+
+    def __init__(self, config=None):
+        if config is not None and not isinstance(config, dict):
+            raise TypeError('Config object must be a dictionary')
+
+        self._config = config or {}
+        self.logger = logging.getLogger(__class__.__module__)
+        default_keys = set(self._default_config.keys())
+        config_keys = set(config.keys()) if config is not None else set([])
+        unexpected_config_keys = list(config_keys.difference(default_keys))
+        using_defaults_for = list(default_keys.difference(config_keys))
+
+        unexpected_config_keys.sort()
+        using_defaults_for.sort()
+
+        for k in unexpected_config_keys:
+            self.logger.warning("%s is an unexpected config value" % k)
+
+        for k in using_defaults_for:
+            self.logger.info("%s key is using default value" % k)
+
+    def get_config(self, key):
+        """
+         Getter for configuration values
+         :param key: Key in the configuration dictionary
+         :return: Current value of the chosen key
+         """
+
+        if key not in self._default_config.keys():
+            raise LongitudeConfigError("%s is not a valid config value. Check your defaults as reference.")
+        try:
+            return self._config[key]
+        except (TypeError, KeyError):
+            try:
+                return self._default_config[key]
+            except KeyError:
+                return None

src/core/common/exceptions.py

@@ -0,0 +1,18 @@
+class LongitudeBaseException(Exception):
+    pass
+
+
+class LongitudeRetriesExceeded(LongitudeBaseException):
+    pass
+
+
+class LongitudeQueryCannotBeExecutedException(LongitudeBaseException):
+    pass
+
+
+class LongitudeWrongQueryException(LongitudeBaseException):
+    pass
+
+
+class LongitudeConfigError(LongitudeBaseException):
+    pass

src/core/data_sources/base.py

@@ -2,22 +2,8 @@
 from typing import Type
 
 from ..caches.base import LongitudeCache
-
-
-class LongitudeBaseException(Exception):
-    pass
-
-
-class LongitudeRetriesExceeded(LongitudeBaseException):
-    pass
-
-
-class LongitudeQueryCannotBeExecutedException(LongitudeBaseException):
-    pass
-
-
-class LongitudeWrongQueryException(LongitudeBaseException):
-    pass
+from ..common.config import LongitudeConfigurable
+from ..common.exceptions import LongitudeRetriesExceeded, LongitudeQueryCannotBeExecutedException
 
 
 class DataSourceQueryConfig:
@@ -31,43 +17,21 @@ def copy(self):
         return DataSourceQueryConfig(self.retries, self.custom)
 
 
-class DataSource:
-    default_config = {}
+class DataSource(LongitudeConfigurable):
 
     def __init__(self, config=None, cache_class: Type[LongitudeCache] = None):
+        super().__init__(config=config)
         self.logger = logging.getLogger(self.__class__.__module__)
         self._default_query_config = DataSourceQueryConfig()
         self._use_cache = True
         self._cache = None
 
-        if config is None:
-            config = {}
-
-        if not isinstance(config, dict):
-            raise TypeError('Config object must be a dictionary')
-
         if cache_class:
             if not issubclass(cache_class, LongitudeCache):
                 raise TypeError('Cache must derive from LongitudeCache or be None')
             else:
                 self._cache = cache_class(config=config.get('cache'))
 
-        default_keys = set(self.default_config.keys())
-        config_keys = set(config.keys())
-        unexpected_config_keys = list(config_keys.difference(default_keys))
-        using_defaults_for = list(default_keys.difference(config_keys))
-
-        unexpected_config_keys.sort()
-        using_defaults_for.sort()
-
-        for k in unexpected_config_keys:
-            self.logger.warning("%s is an unexpected config value" % k)
-
-        for k in using_defaults_for:
-            self.logger.info("%s key is using default value" % k)
-
-        self._config = config
-
     def setup(self):
         if self._cache:
             self._cache.setup()
@@ -105,20 +69,6 @@ def is_ready(self):
         """
         return not self._cache or self._cache.is_ready
 
-    def get_config(self, key: str):
-        """
-        Getter for configuration values
-        :param key: Key in the configuration dictionary
-        :return: Current value of the chosen key
-        """
-        try:
-            return self._config[key]
-        except KeyError:
-            try:
-                return self.default_config[key]
-            except KeyError:
-                return None
-
     def enable_cache(self):
         self._use_cache = True
 

src/core/data_sources/carto.py

@@ -8,7 +8,7 @@
 class CartoDataSource(DataSource):
     SUBDOMAIN_URL_PATTERN = "https://%s.carto.com"
     ON_PREMISE_URL_PATTERN = "https://%s/user/%s"
-    default_config = {
+    _default_config = {
         'api_version': 'v2',
         'uses_batch': False,
         'on_premise_domain': '',

src/core/data_sources/postgres/default.py

@@ -5,7 +5,7 @@
 
 
 class DefaultPostgresDataSource(DataSource):
-    default_config = {
+    _default_config = {
         'host': 'localhost',
         'port': 5432,
         'db': '',

src/core/tests/test_cache_redis.py

@@ -1,14 +1,14 @@
 import redis.exceptions
 from unittest import TestCase, mock
-from ..caches.redis import RedisCache, RedisCacheConfig
+from ..caches.redis import RedisCache
 
 
 @mock.patch('src.core.caches.redis.redis.Redis')
 class TestRedisCache(TestCase):
     cache = None
 
     def setUp(self):
-        self.cache = RedisCache(config=RedisCacheConfig(host='some_host', port=666, db=0, password='some_pass'))
+        self.cache = RedisCache(config={'host': 'some_host', 'port': 666, 'db': 0, 'password': 'some_pass'})
 
     def test_is_ready_if_redis_returns_ping(self, redis_mock):
         redis_mock.return_value.ping.return_value = True

src/core/tests/test_config.py

@@ -0,0 +1,34 @@
+from unittest import TestCase
+
+from src.core.common.exceptions import LongitudeConfigError
+from src.core.common.config import LongitudeConfigurable
+
+
+class TestConfig(TestCase):
+    def test_config(self):
+        # Config must be a dictionary
+        with self.assertRaises(TypeError):
+            LongitudeConfigurable(config=[])
+        with self.assertRaises(TypeError):
+            LongitudeConfigurable(config="")
+        with self.assertRaises(TypeError):
+            LongitudeConfigurable(config=0)
+
+        # Any values can go in the configuration dictionary but not expected ones trigger a warning
+        config = {"some_config_value": 0, "some_another_config_value": "tomato"}
+        with self.assertLogs(level='WARNING') as log_test:
+            ds = LongitudeConfigurable(config)
+            self.assertEqual(log_test.output,
+                             [
+                                 'WARNING:src.core.common.config:some_another_config_value is an unexpected config value',
+                                 'WARNING:src.core.common.config:some_config_value is an unexpected config value'])
+
+        # Values in the config can be retrieved using get_config. If no default or config is defined, None is returned.
+        ds._default_config['some_config_value'] = 42
+        ds._default_config['some_none_value'] = None
+        self.assertEqual(0, ds.get_config('some_config_value'))
+        self.assertEqual(None, ds.get_config('some_none_value'))
+
+        # We do not allow trying to get a config value out of the default keys
+        with self.assertRaises(LongitudeConfigError):
+            self.assertIsNone(ds.get_config('some_random_value_that_does_not_exist_in_config_or_defaults'))

src/core/tests/test_data_source_base.py

@@ -1,6 +1,7 @@
 import os
 from unittest import TestCase, mock
 
+from src.core.common.exceptions import LongitudeConfigError
 from ..caches.base import LongitudeCache
 from ..data_sources.base import DataSource, DataSourceQueryConfig, LongitudeQueryResponse
 
@@ -74,28 +75,7 @@ def test_cache_miss(self, execute_query_mock, parse_response_mock):
         self.assertEqual('normalized response from data source', ds.query('some_query_not_in_cache'))
         parse_response_mock.assert_called_once_with('some response from the server')
 
-    def test_config(self):
-        # Config must be a dictionary
-        with self.assertRaises(TypeError):
-            DataSource([])
-        with self.assertRaises(TypeError):
-            DataSource("")
-        with self.assertRaises(TypeError):
-            DataSource(0)
-
-        # Any values can go in the configuration dictionary but not expected ones trigger a warning
-        config = {"some_config_value": 0, "some_another_config_value": "tomato"}
-        with self.assertLogs(level='WARNING') as log_test:
-            ds = DataSource(config)
-            self.assertEqual(log_test.output,
-                             [
-                                 'WARNING:src.core.data_sources.base:some_another_config_value is an unexpected config value',
-                                 'WARNING:src.core.data_sources.base:some_config_value is an unexpected config value'])
-
-        # Values in the config can be retrieved using get_config. If no default or config is defined, None is returned.
-        self.assertEqual(0, ds.get_config('some_config_value'))
-        self.assertEqual("tomato", ds.get_config('some_another_config_value'))
-        self.assertIsNone(ds.get_config('some_random_value_that_does_not_exist_in_config_or_defaults'))
+
 
     def test_abstract_methods_are_not_implemented(self):
         ds = DataSource({})

src/core/tests/test_data_source_carto.py

@@ -11,13 +11,14 @@ class TestCartoDataSource(TestCase):
     def test_default_configuration_loads(self):
         with self.assertLogs(level='INFO') as log_test:
             carto_ds = CartoDataSource()
+            module_name = 'src.core.common.config'
             self.assertEqual(log_test.output,
-                             ['INFO:src.core.data_sources.carto:api_key key is using default value',
-                              'INFO:src.core.data_sources.carto:api_version key is using default value',
-                              'INFO:src.core.data_sources.carto:cache key is using default value',
-                              'INFO:src.core.data_sources.carto:on_premise_domain key is using default value',
-                              'INFO:src.core.data_sources.carto:user key is using default value',
-                              'INFO:src.core.data_sources.carto:uses_batch key is using default value']
+                             ['INFO:%s:api_key key is using default value' % module_name,
+                              'INFO:%s:api_version key is using default value' % module_name,
+                              'INFO:%s:cache key is using default value' % module_name,
+                              'INFO:%s:on_premise_domain key is using default value' % module_name,
+                              'INFO:%s:user key is using default value' % module_name,
+                              'INFO:%s:uses_batch key is using default value' % module_name]
                              )
 
             self.assertEqual('', carto_ds.get_config('api_key'))

src/samples/carto_sample_with_redis_cache.py

@@ -25,7 +25,7 @@
 import sys
 
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..'))
-from src.core.caches.redis import RedisCache, RedisCacheConfig
+from src.core.caches.redis import RedisCache
 from src.core.data_sources.base import LongitudeRetriesExceeded
 from src.core.data_sources.carto import CartoDataSource
 from src.samples.carto_sample_config import CARTO_API_KEY, CARTO_USER, CARTO_TABLE_NAME
@@ -34,7 +34,7 @@
     config = {
         'api_key': CARTO_API_KEY,
         'user': CARTO_USER,
-        'cache': RedisCacheConfig(password='longitude')
+        'cache': {'password': 'longitude'}
     }
 
     ds = CartoDataSource(config, cache_class=RedisCache)