Repository: mercadona/rele
Pull request #132 (2019-11-27 16:15:44)

Clean up rele.config.setup + Worker() init
This relates to #114 & #119

This makes makes all config variables nullable falling back to standard
google envars, without breaking the current api.

The new apis would look like this is you have
`GOOGLE_APPLICATION_CREDENTIALS` set.

```
rele.config.setup()
```

```
w = Worker([sub1, sub2])
w.run_forever()
```

TBH, I think reading global configs from the environment is easier to
reason about than a singleton so I'd suggest that. Any non globals should
just be passed into the instances on __init__.

### :tophat: What?

Provide a description of what has been implemented.

### :thinking: Why?

Give an explanation of why.

### :link: Related issue

Add related issue's number. Example: Fix #1


hobochild: :eyes: I assume your google creds are set in the CI -> https://travis-ci.org/mercadona/rele/jobs/617799555?utm_medium=notification&utm_source=github_status just a heads up to make sure that service account is locked down.
andrewgy8: You are talking about the Google Cloud ones in the CI? Yeah, I scrambled the values in that file. 
hobochild: > You are talking about the Google Cloud ones in the CI? Yeah, I scrambled the values in that file.

Yea the `travis-ci-prod-2` one.
andrewgy8: 🤔 I have no idea where that value is from. We have no reference to it in our project. I wonder if thats from the machine that Travis is running on?
hobochild: Any word on this @andrewgy8 ?
andrewgy8: Hey @hobochild ! Sorry for the delay. We will be ready to merge this asap. I see there are some conflicts. Could you fix those? I can merge it today and include it in the 0.9.0 release. Thanks!
hobochild: Great, I'll take a look today.

On Thu, Dec 19, 2019 at 9:47 AM Andrew Graham-Yooll <
notifications@github.com> wrote:

> Hey @hobochild <https://github.com/hobochild> ! Sorry for the delay. We
> will be ready to merge this asap. I see there are some conflicts. Could you
> fix those? I can merge it today and include it in the 0.9.0 release. Thanks!
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/mercadona/rele/pull/132?email_source=notifications&email_token=ACIPKDLCIQXJCSE3TEXPVO3QZM7K7A5CNFSM4JSJE45KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHJBMIY#issuecomment-567416355>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ACIPKDK7VSG3B4DXL66TWN3QZM7K7ANCNFSM4JSJE45A>
> .
>

hobochild: Updated @andrewgy8 

Patch:
diff --git a/rele/client.py b/rele/client.py
index cb364eb..53a02d1 100644
--- a/rele/client.py
+++ b/rele/client.py
@@ -6,6 +6,7 @@ from contextlib import suppress
 
 from google.api_core import exceptions
 from google.cloud import pubsub_v1
+import google.auth
 
 from rele.middleware import run_middleware_hook
 
@@ -13,6 +14,15 @@ logger = logging.getLogger(__name__)
 
 USE_EMULATOR = True if os.environ.get("PUBSUB_EMULATOR_HOST") else False
 DEFAULT_ENCODER_PATH = "json.JSONEncoder"
+DEFAULT_ACK_DEADLINE = 60
+
+
+def get_google_defaults():
+    try:
+        credentials, project = google.auth.default()
+        return credentials, project
+    except google.auth.exceptions.DefaultCredentialsError:
+        return None, None
 
 
 class Subscriber:
@@ -26,13 +36,19 @@ class Subscriber:
     :param default_ack_deadline: int Ack Deadline defined in settings
     """
 
-    def __init__(self, gc_project_id, credentials, default_ack_deadline):
-        self._gc_project_id = gc_project_id
-        self._ack_deadline = default_ack_deadline
+    def __init__(self, gc_project_id=None, credentials=None, default_ack_deadline=None):
+
+        if gc_project_id is None or credentials is None:
+            creds, project = get_google_defaults()
+
+        self._gc_project_id = gc_project_id or project
+        self._ack_deadline = default_ack_deadline or DEFAULT_ACK_DEADLINE
+        _credentials = credentials or creds
+
         if USE_EMULATOR:
             self._client = pubsub_v1.SubscriberClient()
         else:
-            self._client = pubsub_v1.SubscriberClient(credentials=credentials)
+            self._client = pubsub_v1.SubscriberClient(credentials=_credentials)
 
     def create_subscription(self, subscription, topic):
         """Handles creating the subscription when it does not exists.
@@ -59,7 +75,7 @@ class Subscriber:
 
     def consume(self, subscription_name, callback, scheduler):
         """Begin listening to topic from the SubscriberClient.
-        
+
         :param subscription_name: str Subscription name
         :param callback: Function which act on a topic message
         :param scheduler: `Thread pool-based scheduler.<https://googleapis.dev/python/pubsub/latest/subscriber/api/scheduler.html?highlight=threadscheduler#google.cloud.pubsub_v1.subscriber.scheduler.ThreadScheduler>`_  # noqa
diff --git a/rele/config.py b/rele/config.py
index 4c576a0..29a8581 100644
--- a/rele/config.py
+++ b/rele/config.py
@@ -1,7 +1,7 @@
 import importlib
 import os
 
-from .client import DEFAULT_ENCODER_PATH
+from .client import DEFAULT_ENCODER_PATH, get_google_defaults, DEFAULT_ACK_DEADLINE
 from .middleware import register_middleware, default_middleware
 from .publishing import init_global_publisher
 from .subscription import Subscription
@@ -19,13 +19,19 @@ class Config:
     """
 
     def __init__(self, setting):
-        self.gc_project_id = setting.get("GC_PROJECT_ID")
-        self.credentials = setting.get("GC_CREDENTIALS")
+        if (
+            setting.get("GC_PROJECT_ID") is None
+            or setting.get("GC_CREDENTIALS") is None
+        ):
+            credentials, project = get_google_defaults()
+
+        self.gc_project_id = setting.get("GC_PROJECT_ID") or project
+        self.credentials = setting.get("GC_CREDENTIALS") or credentials
         self.app_name = setting.get("APP_NAME")
         self.sub_prefix = setting.get("SUB_PREFIX")
         self.middleware = setting.get("MIDDLEWARE", default_middleware)
         self.ack_deadline = setting.get(
-            "ACK_DEADLINE", os.environ.get("DEFAULT_ACK_DEADLINE", 60)
+            "ACK_DEADLINE", os.environ.get("DEFAULT_ACK_DEADLINE", DEFAULT_ACK_DEADLINE)
         )
         self._encoder_path = setting.get("ENCODER_PATH", DEFAULT_ENCODER_PATH)
         self.publisher_timeout = setting.get("PUBLISHER_TIMEOUT", 3.0)
@@ -38,7 +44,10 @@ class Config:
         return getattr(module, class_name)
 
 
-def setup(setting, **kwargs):
+def setup(setting=None, **kwargs):
+    if setting is None:
+        setting = {}
+
     config = Config(setting)
     init_global_publisher(config)
     register_middleware(config, **kwargs)
diff --git a/rele/worker.py b/rele/worker.py
index 567d1a3..08ea6fa 100644
--- a/rele/worker.py
+++ b/rele/worker.py
@@ -22,10 +22,10 @@ class Worker:
     def __init__(
         self,
         subscriptions,
-        gc_project_id,
-        credentials,
-        default_ack_deadline,
-        threads_per_subscription,
+        gc_project_id=None,
+        credentials=None,
+        default_ack_deadline=None,
+        threads_per_subscription=None,
     ):
         self._subscriber = Subscriber(gc_project_id, credentials, default_ack_deadline)
         self._futures = []


Test patch:
diff --git a/tests/test_config.py b/tests/test_config.py
index e7b551f..5fe386a 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -1,7 +1,8 @@
 import json
+import os
 
 import pytest
-
+from unittest.mock import patch
 from rele.config import load_subscriptions_from_paths, Config
 from rele import sub
 
@@ -51,6 +52,7 @@ class TestConfig:
         assert config.credentials == credentials
         assert config.middleware == ["rele.contrib.DjangoDBMiddleware"]
 
+    @patch.dict(os.environ, {"GOOGLE_APPLICATION_CREDENTIALS": ""})
     def test_sets_defaults(self):
         settings = {}
 
@@ -62,3 +64,24 @@ class TestConfig:
         assert config.credentials is None
         assert config.middleware == ["rele.contrib.LoggingMiddleware"]
         assert config.encoder == json.JSONEncoder
+
+    @patch.dict(
+        os.environ,
+        {
+            "GOOGLE_APPLICATION_CREDENTIALS": os.path.dirname(
+                os.path.realpath(__file__)
+            )
+            + "/dummy-pub-sub-credentials.json"
+        },
+    )
+    def test_sets_defaults_pulled_from_env(self, monkeypatch, project_id, credentials):
+        settings = {}
+
+        config = Config(settings)
+
+        assert config.app_name is None
+        assert config.sub_prefix is None
+        assert config.gc_project_id == "rele"
+        assert config.credentials is not None
+        assert config.middleware == ["rele.contrib.LoggingMiddleware"]
+        assert config.encoder == json.JSONEncoder
diff --git a/tests/test_worker.py b/tests/test_worker.py
index 33507ad..f8fb506 100644
--- a/tests/test_worker.py
+++ b/tests/test_worker.py
@@ -1,5 +1,6 @@
 from concurrent import futures
 from unittest.mock import ANY, patch
+import os
 
 import pytest
 
@@ -108,3 +109,21 @@ class TestWorker:
         worker.setup()
 
         assert worker._subscriber._ack_deadline == custom_ack_deadline
+
+    @patch.dict(
+        os.environ,
+        {
+            "GOOGLE_APPLICATION_CREDENTIALS": os.path.dirname(
+                os.path.realpath(__file__)
+            )
+            + "/dummy-pub-sub-credentials.json"
+        },
+    )
+    @pytest.mark.usefixtures("mock_create_subscription")
+    def test_creates_without_config(self):
+        subscriptions = (sub_stub,)
+        worker = Worker(subscriptions)
+        worker.setup()
+
+        assert worker._subscriber._ack_deadline == 60
+        assert worker._subscriber._gc_project_id == "rele"