Repository: mercadona/rele

Issue:
Parametrize subscription ack deadline
For any message for a topic (rele's subscription), use a custom ack deadline value instead of default 10 seconds using `ack_deadline_seconds` parameter in `SubscriberClient.create_subscription()` when creating a subscription.

It has been seen, that in some cases, there is a requirement to expend more than default 10 seconds to consume (late acknowledge) a message.

## Related

https://cloud.google.com/pubsub/docs/reference/rest/v1/projects.subscriptions/modifyAckDeadline

Discussion:
codecov-io: # [Codecov](https://codecov.io/gh/mercadona/rele/pull/49?src=pr&el=h1) Report
> Merging [#49](https://codecov.io/gh/mercadona/rele/pull/49?src=pr&el=desc) into [master](https://codecov.io/gh/mercadona/rele/commit/5553d56a39ba77954e0ceb52970fb02bc14a000a?src=pr&el=desc) will **increase** coverage by `2.06%`.
> The diff coverage is `40%`.

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

```diff
@@            Coverage Diff             @@
##           master      #49      +/-   ##
==========================================
+ Coverage   54.69%   56.75%   +2.06%     
==========================================
  Files           6        6              
  Lines         181      185       +4     
==========================================
+ Hits           99      105       +6     
+ Misses         82       80       -2
```


| [Impacted Files](https://codecov.io/gh/mercadona/rele/pull/49?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [rele/client.py](https://codecov.io/gh/mercadona/rele/pull/49/diff?src=pr&el=tree#diff-cmVsZS9jbGllbnQucHk=) | `43.75% <40%> (+9.65%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/mercadona/rele/pull/49?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/49?src=pr&el=footer). Last update [5553d56...1d9edb4](https://codecov.io/gh/mercadona/rele/pull/49?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/docs/index.rst b/docs/index.rst
index 648a4cb..659092f 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -42,18 +42,10 @@ _______
 .. toctree::
     :maxdepth: 2
 
-    subscription
+    modules/subscription
     modules/client
-
-.. toctree::
-   :maxdepth: 2
-
-   publish
-
-.. toctree::
-   :maxdepth: 2
-
-   worker
+    modules/publish
+    modules/worker
 
 Indices and tables
 ==================
diff --git a/docs/publish.rst b/docs/modules/publish.rst
similarity index 88%
rename from docs/publish.rst
rename to docs/modules/publish.rst
index 1b87350..866fb91 100644
--- a/docs/publish.rst
+++ b/docs/modules/publish.rst
@@ -1,4 +1,4 @@
-.. _publish
+.. _ publish
 
 Publish
 =======
diff --git a/docs/subscription.rst b/docs/modules/subscription.rst
similarity index 85%
rename from docs/subscription.rst
rename to docs/modules/subscription.rst
index ef3f215..f80da4b 100644
--- a/docs/subscription.rst
+++ b/docs/modules/subscription.rst
@@ -1,8 +1,9 @@
-.. _subscription
+.. _ subscription
 
 Subscription
 ============
 
+
 .. automodule:: rele
    :members:
 
diff --git a/docs/worker.rst b/docs/modules/worker.rst
similarity index 88%
rename from docs/worker.rst
rename to docs/modules/worker.rst
index c068446..65b3eae 100644
--- a/docs/worker.rst
+++ b/docs/modules/worker.rst
@@ -1,4 +1,4 @@
-.. _worker
+.. _ worker
 
 Worker
 =======
diff --git a/rele/client.py b/rele/client.py
index dda57fe..f67514d 100644
--- a/rele/client.py
+++ b/rele/client.py
@@ -15,6 +15,7 @@ USE_EMULATOR = True if os.environ.get('PUBSUB_EMULATOR_HOST') else False
 
 
 class Subscriber:
+    DEFAULT_ACK_DEADLINE = 60
 
     def __init__(self, gc_project_id, credentials):
         self._gc_project_id = gc_project_id
@@ -23,14 +24,21 @@ class Subscriber:
         else:
             self._client = pubsub_v1.SubscriberClient(credentials=credentials)
 
-    def create_subscription(self, subscription, topic):
+    def get_default_ack_deadline(self):
+        return int(os.environ.get('DEFAULT_ACK_DEADLINE', self.DEFAULT_ACK_DEADLINE))
+
+    def create_subscription(self, subscription, topic, ack_deadline_seconds=None):
+        ack_deadline_seconds = ack_deadline_seconds or self.get_default_ack_deadline()
+
         subscription_path = self._client.subscription_path(
             self._gc_project_id, subscription)
         topic_path = self._client.topic_path(self._gc_project_id, topic)
 
         with suppress(exceptions.AlreadyExists):
             self._client.create_subscription(
-                name=subscription_path, topic=topic_path)
+                name=subscription_path,
+                topic=topic_path,
+                ack_deadline_seconds=ack_deadline_seconds)
 
     def consume(self, subscription_name, callback):
         subscription_path = self._client.subscription_path(


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index 9277d4f..3a21b7a 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -3,9 +3,25 @@ import concurrent
 from unittest.mock import MagicMock, patch
 from google.cloud.pubsub_v1 import PublisherClient
 from rele import Publisher
+from rele.client import Subscriber
 from tests import settings
 
 
+@pytest.fixture()
+def project_id():
+    return 'test-project-id'
+
+
+@pytest.fixture()
+def credentials():
+    return 'my-credentials'
+
+
+@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,
diff --git a/tests/test_client.py b/tests/test_client.py
index deb0603..dc462bc 100644
--- a/tests/test_client.py
+++ b/tests/test_client.py
@@ -1,6 +1,9 @@
-import pytest
+import os
 import concurrent
-from unittest.mock import ANY
+from unittest.mock import ANY, patch
+
+import pytest
+from google.cloud.pubsub_v1 import SubscriberClient
 
 
 @pytest.mark.usefixtures('publisher', 'time_mock')
@@ -48,3 +51,60 @@ class TestPublisher:
 
         publisher._client.publish.assert_called_with(
             ANY, b'{"foo": "bar"}', published_at=str(published_at))
+
+
+class TestSubscriber:
+    @patch.object(SubscriberClient, 'create_subscription')
+    def test_creates_subscription_with_default_ack_deadline_when_none_provided(
+            self, _mocked_client, project_id, subscriber):
+        expected_subscription = (f'projects/{project_id}/subscriptions/'
+                                 f'test-topic')
+        expected_topic = (f'projects/{project_id}/topics/'
+                          f'{project_id}-test-topic')
+
+        subscriber.create_subscription('test-topic',
+                                       f'{project_id}-test-topic')
+
+        _mocked_client.assert_called_once_with(ack_deadline_seconds=60,
+                                               name=expected_subscription,
+                                               topic=expected_topic)
+
+    @patch.object(SubscriberClient, 'create_subscription')
+    def test_creates_subscription_with_custom_ack_deadline_when_provided(
+            self, _mocked_client, project_id, subscriber):
+        expected_subscription = (f'projects/{project_id}/subscriptions/'
+                                 f'test-topic')
+        expected_topic = (f'projects/{project_id}/topics/'
+                          f'{project_id}-test-topic')
+
+        subscriber.create_subscription('test-topic',
+                                       f'{project_id}-test-topic',
+                                       ack_deadline_seconds=100)
+
+        _mocked_client.assert_called_once_with(ack_deadline_seconds=100,
+                                               name=expected_subscription,
+                                               topic=expected_topic)
+
+    @patch.object(SubscriberClient, 'create_subscription')
+    def test_creates_subscription_with_custom_ack_deadline_from_environment(
+            self, _mocked_client, project_id, subscriber):
+        expected_subscription = (f'projects/{project_id}/subscriptions/'
+                                 f'test-topic')
+        expected_topic = (f'projects/{project_id}/topics/'
+                          f'{project_id}-test-topic')
+
+        with patch.dict(os.environ, {'DEFAULT_ACK_DEADLINE': '200'}):
+            subscriber.create_subscription('test-topic',
+                                           f'{project_id}-test-topic')
+
+        _mocked_client.assert_called_once_with(ack_deadline_seconds=200,
+                                               name=expected_subscription,
+                                               topic=expected_topic)
+
+    def test_get_default_ack_deadline(self, subscriber):
+        assert subscriber.get_default_ack_deadline() == 60
+
+    def test_get_default_ack_deadline_from_environment_variable(
+            self, subscriber):
+        with patch.dict(os.environ, {'DEFAULT_ACK_DEADLINE': '200'}):
+            assert subscriber.get_default_ack_deadline() == 200