Skip to content

Commit

Permalink
Retry TextIt requests if needed. (#1641)
Browse files Browse the repository at this point in the history
Fixes #1567.  I originally used Celery's `autoretry_for` keyword argument here but it was impossible to test (or even to test that I wasn't misspelling `autoretry_for`, as renaming the argument to `LOL` resulted in everything still working) so I reverted to doing things the long way around.  I'm still not particularly convinced this is a good test, or that the retry logic will even work in production, but whatever.
  • Loading branch information
toolness authored Aug 18, 2020
1 parent ec76283 commit 01e0634
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 3 deletions.
14 changes: 11 additions & 3 deletions rapidpro/tasks.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from celery import shared_task
from temba_client.exceptions import TembaHttpError

from .rapidpro_util import get_client_from_settings
from .followup_campaigns import DjangoSettingsFollowupCampaigns
Expand All @@ -8,13 +9,20 @@
# it's really a required argument, but we're allowing it to be optional
# for a bit, because we might still need to process tasks added by old
# versions of the codebase which never supplied this argument.
@shared_task
def trigger_followup_campaign(full_name: str, phone_number: str, campaign_name: str,
@shared_task(
bind=True,
retry_backoff=True,
default_retry_delay=30 * 60
)
def trigger_followup_campaign(self, full_name: str, phone_number: str, campaign_name: str,
locale: str = 'en'):
client = get_client_from_settings()
campaign = DjangoSettingsFollowupCampaigns.get_campaign(campaign_name)

assert client is not None
assert campaign is not None

campaign.add_contact(client, full_name, phone_number, locale=locale)
try:
campaign.add_contact(client, full_name, phone_number, locale=locale)
except TembaHttpError as e:
raise self.retry(exc=e)
23 changes: 23 additions & 0 deletions rapidpro/tests/test_followup_campaigns.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
from unittest.mock import MagicMock
from celery.exceptions import Retry
from temba_client.v2.types import Contact
from temba_client.exceptions import TembaHttpError
from freezegun import freeze_time
import pytest

Expand Down Expand Up @@ -88,3 +90,24 @@ def test_task_works(self, settings, monkeypatch):
campaign.add_contact.assert_called_once()
assert campaign.add_contact.call_args.args[1:] == ('Boop Jones', '5551234567')
assert campaign.add_contact.call_args.kwargs == {'locale': 'en'}

def test_task_retries_on_api_errors(self, settings, monkeypatch):
settings.RAPIDPRO_FOLLOWUP_CAMPAIGN_RH = 'Boop Group,date_of_boop'
settings.RAPIDPRO_API_TOKEN = 'blorp'
dsfc = MagicMock()
campaign = MagicMock()
dsfc.get_campaign.return_value = campaign
campaign.add_contact.side_effect = TembaHttpError("blah")
retry = MagicMock()

def fake_retry(exc):
assert isinstance(exc, TembaHttpError)
raise Retry()

retry.side_effect = fake_retry
monkeypatch.setattr(tasks.trigger_followup_campaign, 'retry', retry)
monkeypatch.setattr(tasks, 'DjangoSettingsFollowupCampaigns', dsfc)
with pytest.raises(Retry):
trigger_followup_campaign_async('Boop Jones', '5551234567', 'RH', 'en')
dsfc.get_campaign.assert_called_once_with('RH')
campaign.add_contact.assert_called_once()

0 comments on commit 01e0634

Please sign in to comment.