Repository: mercadona/rele
Pull request #60 (2019-06-14 12:20:26)

Decouple django settings
### :tophat: What?

Move config to an Object. 
Call rele.setup in apps.py

### :thinking: Why?

Moves dependency of django.conf.settings away from the Middleware and makes config more agnostic, less django dependent.

### :link: Related issue

Addresses #38 


codecov-io: # [Codecov](https://codecov.io/gh/mercadona/rele/pull/60?src=pr&el=h1) Report
> Merging [#60](https://codecov.io/gh/mercadona/rele/pull/60?src=pr&el=desc) into [master](https://codecov.io/gh/mercadona/rele/commit/ca2523b73d2b65c3a7a4f02e8c2bb897c6b7d792?src=pr&el=desc) will **decrease** coverage by `3.64%`.
> The diff coverage is `68.08%`.

[![Impacted file tree graph](https://codecov.io/gh/mercadona/rele/pull/60/graphs/tree.svg?width=650&token=QcryolI2v9&height=150&src=pr)](https://codecov.io/gh/mercadona/rele/pull/60?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master      #60      +/-   ##
==========================================
- Coverage   85.82%   82.17%   -3.65%     
==========================================
  Files          10       10              
  Lines         254      303      +49     
==========================================
+ Hits          218      249      +31     
- Misses         36       54      +18
```


| [Impacted Files](https://codecov.io/gh/mercadona/rele/pull/60?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [rele/apps.py](https://codecov.io/gh/mercadona/rele/pull/60/diff?src=pr&el=tree#diff-cmVsZS9hcHBzLnB5) | `0% <0%> (ø)` | :arrow_up: |
| [rele/contrib/logging\_middleware.py](https://codecov.io/gh/mercadona/rele/pull/60/diff?src=pr&el=tree#diff-cmVsZS9jb250cmliL2xvZ2dpbmdfbWlkZGxld2FyZS5weQ==) | `96% <100%> (-0.16%)` | :arrow_down: |
| [rele/publishing.py](https://codecov.io/gh/mercadona/rele/pull/60/diff?src=pr&el=tree#diff-cmVsZS9wdWJsaXNoaW5nLnB5) | `90% <100%> (ø)` | :arrow_up: |
| [rele/middleware.py](https://codecov.io/gh/mercadona/rele/pull/60/diff?src=pr&el=tree#diff-cmVsZS9taWRkbGV3YXJlLnB5) | `82.05% <100%> (+0.97%)` | :arrow_up: |
| [rele/worker.py](https://codecov.io/gh/mercadona/rele/pull/60/diff?src=pr&el=tree#diff-cmVsZS93b3JrZXIucHk=) | `95.65% <100%> (ø)` | :arrow_up: |
| [rele/management/commands/runrele.py](https://codecov.io/gh/mercadona/rele/pull/60/diff?src=pr&el=tree#diff-cmVsZS9tYW5hZ2VtZW50L2NvbW1hbmRzL3J1bnJlbGUucHk=) | `82.5% <100%> (+10.5%)` | :arrow_up: |
| [rele/config.py](https://codecov.io/gh/mercadona/rele/pull/60/diff?src=pr&el=tree#diff-cmVsZS9jb25maWcucHk=) | `59.25% <57.69%> (-40.75%)` | :arrow_down: |
| [rele/subscription.py](https://codecov.io/gh/mercadona/rele/pull/60/diff?src=pr&el=tree#diff-cmVsZS9zdWJzY3JpcHRpb24ucHk=) | `89.83% <0%> (-5.63%)` | :arrow_down: |
| [rele/client.py](https://codecov.io/gh/mercadona/rele/pull/60/diff?src=pr&el=tree#diff-cmVsZS9jbGllbnQucHk=) | `79.68% <0%> (-5.42%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/mercadona/rele/pull/60?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/mercadona/rele/pull/60?src=pr&el=footer). Last update [ca2523b...ea7db3b](https://codecov.io/gh/mercadona/rele/pull/60?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/rele/__init__.py b/rele/__init__.py
index 1b8e288..85460a3 100644
--- a/rele/__init__.py
+++ b/rele/__init__.py
@@ -1,6 +1,7 @@
 __version__ = '0.3.1'
 
 from .client import Publisher, Subscriber  # noqa
+from .config import setup  # noqa
 from .publishing import publish  # noqa
 from .subscription import Callback, Subscription, sub  # noqa
 from .worker import Worker  # noqa
diff --git a/rele/apps.py b/rele/apps.py
index 4cb8367..5d26874 100644
--- a/rele/apps.py
+++ b/rele/apps.py
@@ -1,5 +1,11 @@
 from django.apps import AppConfig
+from django.conf import settings
+
+import rele.config
 
 
 class ReleConfig(AppConfig):
     name = 'rele'
+
+    def ready(self):
+        rele.config.setup(settings.RELE)
diff --git a/rele/config.py b/rele/config.py
index cb21514..8bac174 100644
--- a/rele/config.py
+++ b/rele/config.py
@@ -1,7 +1,35 @@
-from .middleware import register_middleware
+import importlib
+
+from rele.subscription import Subscription
+from .middleware import register_middleware, default_middleware
 from .publishing import init_global_publisher
 
 
-def setup(gc_project_id, credentials, middleware_paths):
-    init_global_publisher(gc_project_id, credentials)
-    register_middleware(middleware_paths)
+class Config:
+
+    def __init__(self, setting):
+        self.gc_project_id = setting.get('GC_PROJECT_ID')
+        self.credentials = setting.get('GC_CREDENTIALS')
+        self.app_name = setting.get('APP_NAME')
+        self.sub_prefix = setting.get('SUB_PREFIX')
+        self.middleware = setting.get('MIDDLEWARE', default_middleware)
+
+
+def setup(setting):
+    config = Config(setting)
+    init_global_publisher(config)
+    register_middleware(config)
+    return config
+
+
+def load_subscriptions_from_paths(sub_module_paths, sub_prefix=None):
+    subscriptions = []
+    for sub_module_path in sub_module_paths:
+        sub_module = importlib.import_module(sub_module_path)
+        for attr_name in dir(sub_module):
+            attribute = getattr(sub_module, attr_name)
+            if isinstance(attribute, Subscription):
+                if sub_prefix and not attribute.prefix:
+                    attribute.set_prefix(sub_prefix)
+                subscriptions.append(attribute)
+    return subscriptions
diff --git a/rele/contrib/logging_middleware.py b/rele/contrib/logging_middleware.py
index eaa0c78..be6e71c 100644
--- a/rele/contrib/logging_middleware.py
+++ b/rele/contrib/logging_middleware.py
@@ -1,8 +1,6 @@
 import logging
 import time
 
-from django.conf import settings
-
 from rele.middleware import BaseMiddleware
 
 
@@ -10,10 +8,10 @@ class LoggingMiddleware(BaseMiddleware):
 
     def __init__(self):
         self._logger = None
-        self._app_name = settings.BASE_DIR.split('/')[-1]
 
-    def setup(self):
+    def setup(self, config):
         self._logger = logging.getLogger(__name__)
+        self._app_name = config.app_name
 
     def _build_data_metrics(self, subscription, status, start_processing_time=None):
         result = {
diff --git a/rele/management/commands/runrele.py b/rele/management/commands/runrele.py
index fc09b07..5f2e6d7 100644
--- a/rele/management/commands/runrele.py
+++ b/rele/management/commands/runrele.py
@@ -1,4 +1,3 @@
-import importlib
 import logging
 import signal
 import time
@@ -8,26 +7,24 @@ from django.conf import settings
 from django.core.management import BaseCommand
 from django.utils.module_loading import module_has_submodule
 
-from rele import config, Worker
+from rele import Worker
+import rele
 
 logger = logging.getLogger(__name__)
 
 
 class Command(BaseCommand):
-    help = 'Start subscriber threads to consume rele topics.'
+    help = 'Start subscriber threads to consume Relé topics.'
 
     def handle(self, *args, **options):
-        config.setup(settings.RELE_GC_PROJECT_ID,
-                     settings.RELE_GC_CREDENTIALS,
-                     settings.RELE_MIDDLEWARE)
-        subs = self._autodiscover_subs()
+        subs = self._autodiscover_subs(settings.RELE['SUB_PREFIX'])
         self.stdout.write(f'Configuring worker with {len(subs)} '
                           f'subscription(s)...')
         for sub in subs:
             self.stdout.write(f'  {sub}')
-        worker = Worker(settings.RELE_GC_PROJECT_ID,
-                        settings.RELE_GC_CREDENTIALS,
-                        subs)
+        worker = Worker(subs,
+                        settings.RELE['GC_PROJECT_ID'],
+                        settings.RELE['GC_CREDENTIALS'])
         worker.setup()
         worker.start()
 
@@ -48,17 +45,8 @@ class Command(BaseCommand):
                 self.stdout.write(" * Discovered subs module: %r" % module)
         return subs_modules
 
-    def _autodiscover_subs(self):
-        subscriptions = []
-        for sub_module_path in self._discover_subs_modules():
-            sub_module = importlib.import_module(sub_module_path)
-            for attr_name in dir(sub_module):
-                attribute = getattr(sub_module, attr_name)
-                if isinstance(attribute, rele.Subscription):
-                    if settings.RELE_PREFIX and not attribute.prefix:
-                        attribute.set_prefix(settings.RELE_SUB_PREFIX)
-                    subscriptions.append(attribute)
-        return subscriptions
+    def _autodiscover_subs(self, sub_prefix):
+        return rele.config.load_subscriptions_from_paths(self._discover_subs_modules(), sub_prefix)
 
     def _wait_forever(self):
         self.stdout.write('Consuming subscriptions...')
diff --git a/rele/middleware.py b/rele/middleware.py
index 7fe2a6a..25faca2 100644
--- a/rele/middleware.py
+++ b/rele/middleware.py
@@ -3,7 +3,11 @@ import importlib
 _middlewares = []
 
 
-def register_middleware(paths):
+default_middleware = ['rele.contrib.LoggingMiddleware']
+
+
+def register_middleware(config):
+    paths = config.middleware
     global _middlewares
     _middlewares = []
     for path in paths:
@@ -11,7 +15,7 @@ def register_middleware(paths):
         module_path = '.'.join(module_parts)
         module = importlib.import_module(module_path)
         middleware = getattr(module, middleware_class)()
-        middleware.setup()
+        middleware.setup(config)
         _middlewares.append(middleware)
 
 
@@ -22,7 +26,7 @@ def run_middleware_hook(hook_name, *args, **kwargs):
 
 class BaseMiddleware:
 
-    def setup(self):
+    def setup(self, config):
         pass
 
     def pre_publish(self, topic, data, attrs):
diff --git a/rele/publishing.py b/rele/publishing.py
index f1616b7..21df7a4 100644
--- a/rele/publishing.py
+++ b/rele/publishing.py
@@ -3,10 +3,10 @@ from .client import Publisher
 _publisher = None
 
 
-def init_global_publisher(gc_project_id, credentials):
+def init_global_publisher(config):
     global _publisher
     if not _publisher:
-        _publisher = Publisher(gc_project_id, credentials)
+        _publisher = Publisher(config.gc_project_id, config.credentials)
     return _publisher
 
 
diff --git a/rele/worker.py b/rele/worker.py
index bd5dc82..b26a702 100644
--- a/rele/worker.py
+++ b/rele/worker.py
@@ -16,7 +16,7 @@ class Worker:
 
     :param subscriptions: list, List of :class:`~rele.subscription.Subscription`
     """
-    def __init__(self, gc_project_id, credentials, subscriptions):
+    def __init__(self, subscriptions, gc_project_id, credentials):
         self._subscriber = Subscriber(gc_project_id, credentials)
         self._futures = []
         self._subscriptions = subscriptions
diff --git a/setup.py b/setup.py
index abc7484..49741c4 100644
--- a/setup.py
+++ b/setup.py
@@ -43,6 +43,7 @@ setup(
     url='https://github.com/mercadona/rele',
     packages=[
         'rele',
+        'rele.contrib',
         'rele.management',
         'rele.management.commands',
     ],


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index 8793207..14aac13 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -2,10 +2,11 @@ import pytest
 import concurrent
 from unittest.mock import MagicMock, patch
 from google.cloud.pubsub_v1 import PublisherClient
+
+from rele.config import Config
 from rele import Publisher
 from rele.client import Subscriber
 from rele.middleware import register_middleware
-from tests import settings
 
 
 @pytest.fixture
@@ -18,15 +19,26 @@ def credentials():
     return 'my-credentials'
 
 
+@pytest.fixture
+def config(credentials, project_id):
+    return Config({
+        'APP_NAME': 'rele',
+        'SUB_PREFIX': 'rele',
+        'GC_PROJECT_ID': project_id,
+        'GC_CREDENTIALS': credentials,
+        'MIDDLEWARE': ['rele.contrib.LoggingMiddleware']
+    })
+
+
 @pytest.fixture
 def subscriber(project_id, credentials):
     return Subscriber(project_id, credentials)
 
 
-@pytest.fixture(scope='class')
-def publisher():
-    publisher = Publisher(settings.RELE_GC_PROJECT_ID,
-                          settings.RELE_GC_CREDENTIALS)
+@pytest.fixture
+def publisher(config):
+    publisher = Publisher(config.gc_project_id,
+                          config.credentials)
     publisher._client = MagicMock(spec=PublisherClient)
     publisher._client.publish.return_value = concurrent.futures.Future()
 
@@ -46,5 +58,5 @@ def time_mock(published_at):
 
 
 @pytest.fixture(autouse=True)
-def default_middleware():
-    register_middleware(['rele.contrib.LoggingMiddleware'])
+def default_middleware(config):
+    register_middleware(config=config)
diff --git a/tests/settings.py b/tests/settings.py
index 7208beb..46e821e 100644
--- a/tests/settings.py
+++ b/tests/settings.py
@@ -24,17 +24,18 @@ SITE_ID = 1
 
 MIDDLEWARE_CLASSES = ()
 
-# Google Cloud
-RELE_GC_PROJECT_ID = 'SOME-PROJECT-ID'
-RELE_GC_CREDENTIALS = service_account.Credentials.from_service_account_file(
-    f'{BASE_DIR}/tests/dummy-pub-sub-credentials.json'
-)
-RELE_SUB_PREFIX = 'rele'
-
-RELE_MIDDLEWARE = [
-    'rele.contrib.LoggingMiddleware',
-    'rele.contrib.DjangoDBMiddleware',
-]
+RELE = {
+    'APP_NAME': 'test-rele',
+    'GC_PROJECT_ID': 'SOME-PROJECT-ID',
+    'GC_CREDENTIALS': service_account.Credentials.from_service_account_file(
+        f'{BASE_DIR}/tests/dummy-pub-sub-credentials.json'
+    ),
+    'SUB_PREFIX': 'rele',
+    'MIDDLEWARE': [
+        'rele.contrib.LoggingMiddleware',
+        'rele.contrib.DjangoDBMiddleware',
+    ],
+}
 
 logging_config.dictConfig({
     'version': 1,
diff --git a/tests/test_publishing.py b/tests/test_publishing.py
index 7c4d8d5..ff9a33b 100644
--- a/tests/test_publishing.py
+++ b/tests/test_publishing.py
@@ -2,17 +2,14 @@ from unittest.mock import MagicMock, patch
 
 from rele import Publisher, publishing
 
-from . import settings
-
 
 class TestPublish:
 
     @patch('rele.publishing.Publisher', autospec=True)
     def test_creates_global_publisher_when_published_called(
-            self, mock_publisher):
+            self, mock_publisher, config):
         mock_publisher.return_value = MagicMock(spec=Publisher)
-        publishing.init_global_publisher(settings.RELE_GC_PROJECT_ID,
-                                         settings.RELE_GC_CREDENTIALS)
+        publishing.init_global_publisher(config)
         message = {'foo': 'bar'}
         publishing.publish(
             topic='order-cancelled', data=message, myattr='hello')
diff --git a/tests/test_subscription.py b/tests/test_subscription.py
index e3ffef7..60d928f 100644
--- a/tests/test_subscription.py
+++ b/tests/test_subscription.py
@@ -160,8 +160,9 @@ class TestCallback:
         assert success_log.message == "<class 'float'>"
 
     def test_old_django_connections_closed_when_middleware_is_used(
-            self, mock_close_old_connections, message_wrapper):
-        register_middleware(['rele.contrib.DjangoDBMiddleware'])
+            self, mock_close_old_connections, message_wrapper, config):
+        config.middleware = ['rele.contrib.DjangoDBMiddleware']
+        register_middleware(config)
         callback = Callback(sub_stub)
         res = callback(message_wrapper)
 
diff --git a/tests/test_worker.py b/tests/test_worker.py
index b8206c5..eee5735 100644
--- a/tests/test_worker.py
+++ b/tests/test_worker.py
@@ -5,8 +5,6 @@ import pytest
 from rele import Subscriber, Worker, sub
 from rele.middleware import register_middleware
 
-from . import settings
-
 
 @sub(topic='some-cool-topic', prefix='rele')
 def sub_stub(data, **kwargs):
@@ -17,11 +15,9 @@ class TestWorker:
 
     @patch.object(Subscriber, 'consume')
     def test_start_subscribes_and_saves_futures_when_subscriptions_given(
-            self, mock_consume):
+            self, mock_consume, project_id, credentials):
         subscriptions = (sub_stub,)
-        worker = Worker(settings.RELE_GC_PROJECT_ID,
-                        settings.RELE_GC_CREDENTIALS,
-                        subscriptions)
+        worker = Worker(subscriptions, project_id, credentials)
         worker.start()
 
         mock_consume.assert_called_once_with(
@@ -31,11 +27,9 @@ class TestWorker:
 
     @patch.object(Subscriber, 'create_subscription')
     def test_setup_creates_subscription_when_topic_given(
-            self, mock_create_subscription):
+            self, mock_create_subscription, project_id, credentials):
         subscriptions = (sub_stub,)
-        worker = Worker(settings.RELE_GC_PROJECT_ID,
-                        settings.RELE_GC_CREDENTIALS,
-                        subscriptions)
+        worker = Worker(subscriptions, project_id, credentials)
         worker.setup()
 
         topic = 'some-cool-topic'
@@ -43,12 +37,12 @@ class TestWorker:
         mock_create_subscription.assert_called_once_with(subscription, topic)
 
     @patch('rele.contrib.django_db_middleware.db.connections.close_all')
-    def test_stop_closes_db_connections(self, mock_db_close_all):
-        register_middleware(['rele.contrib.DjangoDBMiddleware'])
+    def test_stop_closes_db_connections(
+            self, mock_db_close_all, config, project_id, credentials):
+        config.middleware = ['rele.contrib.DjangoDBMiddleware']
+        register_middleware(config=config)
         subscriptions = (sub_stub,)
-        worker = Worker(settings.RELE_GC_PROJECT_ID,
-                        settings.RELE_GC_CREDENTIALS,
-                        subscriptions)
+        worker = Worker(subscriptions, project_id, credentials)
 
         with pytest.raises(SystemExit):
             worker.stop()