Skip to content

Commit

Permalink
Merge pull request #11 from maykinmedia/issue-10-avoid-runtimeerror
Browse files Browse the repository at this point in the history
[#10] Avoid raising a RuntimeError when not having notifications conf…
  • Loading branch information
sergei-maertens authored Aug 8, 2023
2 parents 25c99c2 + 9f41b50 commit 7abd32e
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 4 deletions.
2 changes: 1 addition & 1 deletion README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ Installation
Requirements
------------

* Python 3.7 or above
* Python 3.8 or above
* setuptools 30.3.0 or above
* Django 3.2 or newer
* Celery 5.0 or newer setup with one worker deployed
Expand Down
2 changes: 2 additions & 0 deletions docs/quickstart.rst
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ Three additional settings are available:

* ``NOTIFICATIONS_DISABLED``: a boolean, default ``False``. Set to ``True`` to
completely disable the sending of notifications.
* ``NOTIFICATIONS_GUARANTEE_DELIVERY``: a boolean, default ``True``. Set to ``False`` to
not raise a RuntimeError when the Notifications API is unconfigured.
* ``IS_HTTPS``: a boolean, default ``True``. Set to ``False`` to indicate that
no HTTPS is being used.

Expand Down
2 changes: 2 additions & 0 deletions notifications_api_common/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

IS_HTTPS = True

NOTIFICATIONS_GUARANTEE_DELIVERY = True


def get_setting(name: str) -> Any:
this_module = sys.modules[__name__]
Expand Down
11 changes: 8 additions & 3 deletions notifications_api_common/viewsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,16 @@ def notify(
# build the content of the notification
message = self.construct_message(data, instance=instance)

# build the client from the singleton config. This will raise an
# exception if the config is not complete. We want this to hard-fail!
# build the client from the singleton config.
# This will raise an exception if the config is not complete unless
# NOTIFICATIONS_GUARANTEE_DELIVERY is explicitly set to False
client = NotificationsConfig.get_client()
if client is None:
raise RuntimeError("Could not build a client for Notifications API")
msg = "Not notifying, Notifications API configuration is broken or absent."
logger.warning(msg)
if get_setting("NOTIFICATIONS_GUARANTEE_DELIVERY"):
raise RuntimeError(msg)
return

# We've performed all the work that can raise uncaught exceptions that we can
# still put inside an atomic transaction block. Next, we schedule the actual
Expand Down
28 changes: 28 additions & 0 deletions tests/test_send_message.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from pathlib import Path
from unittest.mock import patch

from django.test import override_settings
from django.urls import reverse

import pytest
Expand Down Expand Up @@ -116,3 +117,30 @@ def test_task_send_notification_catch_500(

with pytest.raises(NotificationException):
send_notification({"foo": "bar"})


@freeze_time("2022-01-01")
@pytest.mark.django_db(transaction=True)
def test_api_create_person_unconfigured(api_client, notifications_config):
"""Test the default behavior of NOTIFICATIONS_GUARANTEE_DELIVERY=True"""
notifications_config.delete()
url = reverse("person-list")
data = {"name": "John", "address_street": "Grotestraat", "address_number": "1"}

with pytest.raises(RuntimeError):
response = api_client.post(url, data)

assert Person.objects.count() == 0


@freeze_time("2022-01-01")
@pytest.mark.django_db(transaction=True)
@override_settings(NOTIFICATIONS_GUARANTEE_DELIVERY=False)
def test_api_create_person_unconfigured(api_client, notifications_config):
notifications_config.delete()
url = reverse("person-list")
data = {"name": "John", "address_street": "Grotestraat", "address_number": "1"}

response = api_client.post(url, data)

assert Person.objects.count() == 1

0 comments on commit 7abd32e

Please sign in to comment.