Memoize config per service (#63)

* Memoizes config file per service
* Updates dependencies

Author: miguelgrubin

Pipfile

@@ -9,7 +9,7 @@ python-json-logger = ">=0.1.10"
 pyyaml = ">=5.1.2"
 anyconfig = ">=0.9.8"
 swagger-ui-bundle = ">=0.0.2"
-connexion = {extras = ["swagger-ui"],version = ">=2.2.0"}
+connexion = {extras = ["swagger-ui"],version = "==2.4.0"}
 jaeger-client = "==4.1.0"
 flask-opentracing = "*"
 opentracing = ">=2.1"

Pipfile.lock

@@ -2,28 +2,14 @@
 from pyms.exceptions import ServiceDoesNotExistException
 
 
-class Config:
-    service = None
-    _config = False
+__service_configs = {}
 
-    def __init__(self):
-        pass
 
-    def config(self, *args, **kwargs):
-        """Set the configuration, if our yaml file is like:
-        myservice:
-          myservice1:
-           myvar1
-        and we want to get the configuration of service1, our self.service will be "myservice.myservice1"
-        """
-        if not self._config:
-            self._config = ConfFile(*args, **kwargs)
-        if not self.service:
-            raise ServiceDoesNotExistException("Service not defined")
-        return getattr(self._config, self.service)
-
-
-def get_conf(service=None, *args, **kwargs):
-    config = Config()
-    config.service = service
-    return config.config(*args, **kwargs)
+def get_conf(*args, **kwargs):
+    service = kwargs.pop('service', None)
+    memoize = kwargs.pop('memoize', True)
+    if not service:
+        raise ServiceDoesNotExistException("Service not defined")
+    if not memoize or service not in __service_configs:
+        __service_configs[service] = ConfFile(*args, **kwargs)
+    return getattr(__service_configs[service], service)

pyms/config/conf.py

@@ -44,7 +44,7 @@ class Microservice(metaclass=SingletonMeta):
     def __init__(self, *args, **kwargs):
         self.service = kwargs.get("service", os.environ.get(SERVICE_ENVIRONMENT, "ms"))
         self.path = os.path.dirname(kwargs.get("path", __file__))
-        self.config = get_conf(service=self.service)
+        self.config = get_conf(service=self.service, memoize=self._singleton)
         self.init_services()
 
     def init_services(self):

pyms/flask/app/create_app.py

@@ -25,7 +25,7 @@ class ServicesManager:
 
     def __init__(self, service=None):
         self.service = (service if service else SERVICE_BASE)
-        self.config = get_conf(service=self.service, empty_init=True)
+        self.config = get_conf(service=self.service, empty_init=True, memoize=False)
 
     def get_services(self):
         return ((k, self.get_service(k)) for k in self.config.__dict__.keys() if k not in ['empty_init', ])

pyms/flask/services/driver.py

@@ -1,8 +1,9 @@
 import logging
 import os
 import unittest
+from unittest import mock
 
-from pyms.config.conf import Config
+from pyms.config.conf import get_conf
 from pyms.config.confile import ConfFile
 from pyms.constants import CONFIGMAP_FILE_ENVIRONMENT, LOGGER_NAME
 from pyms.exceptions import AttrDoesNotExistException, ConfigDoesNotFoundException, ServiceDoesNotExistException
@@ -67,6 +68,14 @@ def test_equal_instances_ok2(self):
         config2 = {"test_1": {"test_1_1": "a", "test_1_2": "b"}}
         self.assertEqual(config1, config2)
 
+    def test_equal_instances_ko(self):
+        config = ConfFile(config={"test-1": {"test-1-1": "a"}})
+        no_valid_type = ConfigDoesNotFoundException
+
+        result = config == no_valid_type
+
+        self.assertEqual(result, False)
+
     def test_dictionary_attribute_not_exists(self):
         config = ConfFile(config={"test-1": "a"})
         with self.assertRaises(AttrDoesNotExistException):
@@ -89,22 +98,6 @@ def test_example_test_json_file(self):
         self.assertEqual(config.my_ms.test_var, "general")
 
 
-class ConfServiceTests(unittest.TestCase):
-
-    def test_config_with_service(self):
-        class MyService(Config):
-            service = "service"
-
-        config = MyService()
-        configuration = config.config(config={"service": {"service1": "a", "service2": "b"}})
-        self.assertEqual(configuration.service1, "a")
-
-    def test_config_with_service_not_exist(self):
-        config = Config()
-        with self.assertRaises(ServiceDoesNotExistException):
-            configuration = config.config(config={"service": {"service1": "a", "service2": "b"}})
-
-
 class ConfNotExistTests(unittest.TestCase):
     def test_empty_conf(self):
         config = ConfFile(empty_init=True)
@@ -119,5 +112,39 @@ def test_empty_conf_three_levels(self):
         self.assertEqual(config.my_ms.level_two.level_three, {})
 
 
-if __name__ == '__main__':
-    unittest.main()
+
+class GetConfig(unittest.TestCase):
+    BASE_DIR = os.path.dirname(os.path.abspath(__file__))
+
+    def setUp(self):
+        os.environ[CONFIGMAP_FILE_ENVIRONMENT] = os.path.join(self.BASE_DIR, "config-tests.yml")
+
+    def tearDown(self):
+        del os.environ[CONFIGMAP_FILE_ENVIRONMENT]
+
+    def test_default(self):
+        config = get_conf(service="my-ms")
+
+        assert config.APP_NAME == "Python Microservice"
+        assert config.subservice1.test == "input"
+
+    @mock.patch('pyms.config.conf.ConfFile')
+    def test_memoized(self, mock_confile):
+        mock_confile.pyms = {}
+        get_conf(service="pyms")
+        get_conf(service="pyms")
+
+        mock_confile.assert_called_once()
+
+    @mock.patch('pyms.config.conf.ConfFile')
+    def test_without_memoize(self, mock_confile):
+        mock_confile.pyms = {}
+        get_conf(service="pyms", memoize=False)
+        get_conf(service="pyms", memoize=False)
+
+        assert mock_confile.call_count == 2
+
+    @mock.patch('pyms.config.conf.ConfFile')
+    def test_without_params(self, mock_confile):
+        with self.assertRaises(ServiceDoesNotExistException):
+            get_conf()

tests/test_config.py

@@ -1,5 +1,6 @@
 import os
 import unittest
+from unittest import mock
 
 import pytest
 from flask import current_app
@@ -38,6 +39,12 @@ def test_home(self):
         response = self.client.get('/')
         self.assertEqual(404, response.status_code)
 
+    def test_healthcheck(self):
+        response = self.client.get('/healthcheck')
+        self.assertEqual(b"OK", response.data)
+        self.assertEqual(200, response.status_code)
+
+
 
 class MicroserviceTest(unittest.TestCase):
     """
@@ -68,22 +75,28 @@ def test_import_config_without_create_app(self):
         ms1 = MyMicroservice(service="my-ms", path=__file__, override_instance=True)
         self.assertEqual(ms1.config.subservice1, config().subservice1)
 
+    def test_config_singleton(self):
+        conf_one = config().subservice1
+        conf_two = config().subservice1
+
+        assert conf_one is conf_two
+
 
 @pytest.mark.parametrize("payload, configfile, status_code", [
     (
-            "Python Microservice",
-            "config-tests.yml",
-            200
+        "Python Microservice",
+        "config-tests.yml",
+        200
     ),
     (
-            "Python Microservice With Flask",
-            "config-tests-flask.yml",
-            404
+        "Python Microservice With Flask",
+        "config-tests-flask.yml",
+        404
     ),
     (
-            "Python Microservice With Flask and Lightstep",
-            "config-tests-flask-trace-lightstep.yml",
-            200
+        "Python Microservice With Flask and Lightstep",
+        "config-tests-flask-trace-lightstep.yml",
+        200
     )
 ])
 def test_configfiles(payload, configfile, status_code):

tests/test_flask.py

@@ -268,6 +268,35 @@ def test_propagate_headers_propagate_no_override(self):
 
         self.assertEqual(expected_headers, headers)
 
+    def test_propagate_headers_on_get(self):
+        url = "http://www.my-site.com/users"
+        mock_headers = {
+            'A': 'b',
+        }
+        self.request.propagate_headers = unittest.mock.Mock()
+        self.request.propagate_headers.return_value = mock_headers
+        with self.app.test_request_context(
+                '/tests/', data={'format': 'short'}, headers=mock_headers):
+            self.request.get(url, propagate_headers=True)
+
+        self.request.propagate_headers.assert_called_once_with({})
+
+    def test_propagate_headers_on_get_with_headers(self):
+        url = "http://www.my-site.com/users"
+        mock_headers = {
+            'A': 'b',
+        }
+        get_headers = {
+            'C': 'd',
+        }
+        self.request.propagate_headers = unittest.mock.Mock()
+        self.request.propagate_headers.return_value = mock_headers
+        with self.app.test_request_context(
+                '/tests/', data={'format': 'short'}, headers=mock_headers):
+            self.request.get(url, headers=get_headers, propagate_headers=True)
+
+        self.request.propagate_headers.assert_called_once_with(get_headers)
+
     @requests_mock.Mocker()
     def test_retries_with_500(self, mock_request):
         url = 'http://localhost:9999'