mirror of
https://github.com/GSA/notifications-api.git
synced 2026-02-04 02:11:11 -05:00
update broadcast provider message status on success/error
so we can distinguish errorring messages that are currently retrying from those that sent succesfully.
This commit is contained in:
@@ -9,8 +9,12 @@ from celery.exceptions import MaxRetriesExceededError
|
|||||||
from app import cbc_proxy_client, db, notify_celery
|
from app import cbc_proxy_client, db, notify_celery
|
||||||
from app.clients.cbc_proxy import CBCProxyFatalException, CBCProxyRetryableException
|
from app.clients.cbc_proxy import CBCProxyFatalException, CBCProxyRetryableException
|
||||||
from app.config import QueueNames
|
from app.config import QueueNames
|
||||||
from app.models import BroadcastEventMessageType, BroadcastProvider
|
from app.models import BroadcastEventMessageType, BroadcastProvider, BroadcastProviderMessageStatus
|
||||||
from app.dao.broadcast_message_dao import dao_get_broadcast_event_by_id, create_broadcast_provider_message
|
from app.dao.broadcast_message_dao import (
|
||||||
|
dao_get_broadcast_event_by_id,
|
||||||
|
create_broadcast_provider_message,
|
||||||
|
update_broadcast_provider_message_status
|
||||||
|
)
|
||||||
|
|
||||||
from app.utils import format_sequential_number
|
from app.utils import format_sequential_number
|
||||||
|
|
||||||
@@ -138,12 +142,20 @@ def send_broadcast_provider_message(self, broadcast_event_id, provider):
|
|||||||
# (because the message has expired)
|
# (because the message has expired)
|
||||||
check_provider_message_should_retry(broadcast_provider_message)
|
check_provider_message_should_retry(broadcast_provider_message)
|
||||||
|
|
||||||
|
# TODO: Decide whether to set to TECHNICAL_FAILURE or ERROR based on response codes from cbc proxy
|
||||||
|
update_broadcast_provider_message_status(
|
||||||
|
broadcast_provider_message,
|
||||||
|
status=BroadcastProviderMessageStatus.TECHNICAL_FAILURE
|
||||||
|
)
|
||||||
|
|
||||||
self.retry(
|
self.retry(
|
||||||
exc=exc,
|
exc=exc,
|
||||||
countdown=get_retry_delay(self.request.retries),
|
countdown=get_retry_delay(self.request.retries),
|
||||||
queue=QueueNames.BROADCASTS,
|
queue=QueueNames.BROADCASTS,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
update_broadcast_provider_message_status(broadcast_provider_message, status=BroadcastProviderMessageStatus.ACK)
|
||||||
|
|
||||||
|
|
||||||
@notify_celery.task(name='trigger-link-test')
|
@notify_celery.task(name='trigger-link-test')
|
||||||
def trigger_link_test(provider):
|
def trigger_link_test(provider):
|
||||||
|
|||||||
@@ -61,3 +61,8 @@ def create_broadcast_provider_message(broadcast_event, provider):
|
|||||||
db.session.add(provider_message_number)
|
db.session.add(provider_message_number)
|
||||||
db.session.commit()
|
db.session.commit()
|
||||||
return provider_message
|
return provider_message
|
||||||
|
|
||||||
|
|
||||||
|
@transactional
|
||||||
|
def update_broadcast_provider_message_status(broadcast_provider_message, status):
|
||||||
|
broadcast_provider_message.status = status
|
||||||
|
|||||||
@@ -3,7 +3,7 @@ from datetime import datetime
|
|||||||
from unittest.mock import call, ANY
|
from unittest.mock import call, ANY
|
||||||
|
|
||||||
from freezegun import freeze_time
|
from freezegun import freeze_time
|
||||||
from celery.exceptions import MaxRetriesExceededError
|
from celery.exceptions import MaxRetriesExceededError, Retry
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
from app.models import (
|
from app.models import (
|
||||||
@@ -147,7 +147,7 @@ def test_send_broadcast_provider_message_sends_data_correctly(
|
|||||||
send_broadcast_provider_message(provider=provider, broadcast_event_id=str(event.id))
|
send_broadcast_provider_message(provider=provider, broadcast_event_id=str(event.id))
|
||||||
|
|
||||||
broadcast_provider_message = event.get_provider_message(provider)
|
broadcast_provider_message = event.get_provider_message(provider)
|
||||||
assert broadcast_provider_message.status == BroadcastProviderMessageStatus.SENDING
|
assert broadcast_provider_message.status == BroadcastProviderMessageStatus.ACK
|
||||||
|
|
||||||
mock_create_broadcast.assert_called_once_with(
|
mock_create_broadcast.assert_called_once_with(
|
||||||
identifier=str(broadcast_provider_message.id),
|
identifier=str(broadcast_provider_message.id),
|
||||||
@@ -280,9 +280,9 @@ def test_send_broadcast_provider_message_works_if_we_retried_previously(mocker,
|
|||||||
assert len(event.provider_messages) == 1
|
assert len(event.provider_messages) == 1
|
||||||
|
|
||||||
broadcast_provider_message = event.get_provider_message('ee')
|
broadcast_provider_message = event.get_provider_message('ee')
|
||||||
# TODO: Should be ACK, and should have an updated_at
|
|
||||||
assert broadcast_provider_message.status == BroadcastProviderMessageStatus.TECHNICAL_FAILURE
|
assert broadcast_provider_message.status == BroadcastProviderMessageStatus.ACK
|
||||||
assert broadcast_provider_message.updated_at is None
|
assert broadcast_provider_message.updated_at is not None
|
||||||
|
|
||||||
mock_create_broadcast.assert_called_once_with(
|
mock_create_broadcast.assert_called_once_with(
|
||||||
identifier=str(broadcast_provider_message.id),
|
identifier=str(broadcast_provider_message.id),
|
||||||
@@ -374,7 +374,7 @@ def test_send_broadcast_provider_message_sends_update_with_references(
|
|||||||
send_broadcast_provider_message(provider=provider, broadcast_event_id=str(update_event.id))
|
send_broadcast_provider_message(provider=provider, broadcast_event_id=str(update_event.id))
|
||||||
|
|
||||||
broadcast_provider_message = update_event.get_provider_message(provider)
|
broadcast_provider_message = update_event.get_provider_message(provider)
|
||||||
assert broadcast_provider_message.status == BroadcastProviderMessageStatus.SENDING
|
assert broadcast_provider_message.status == BroadcastProviderMessageStatus.ACK
|
||||||
|
|
||||||
mock_update_broadcast.assert_called_once_with(
|
mock_update_broadcast.assert_called_once_with(
|
||||||
identifier=str(broadcast_provider_message.id),
|
identifier=str(broadcast_provider_message.id),
|
||||||
@@ -429,7 +429,7 @@ def test_send_broadcast_provider_message_sends_cancel_with_references(
|
|||||||
send_broadcast_provider_message(provider=provider, broadcast_event_id=str(cancel_event.id))
|
send_broadcast_provider_message(provider=provider, broadcast_event_id=str(cancel_event.id))
|
||||||
|
|
||||||
broadcast_provider_message = cancel_event.get_provider_message(provider)
|
broadcast_provider_message = cancel_event.get_provider_message(provider)
|
||||||
assert broadcast_provider_message.status == BroadcastProviderMessageStatus.SENDING
|
assert broadcast_provider_message.status == BroadcastProviderMessageStatus.ACK
|
||||||
|
|
||||||
mock_cancel_broadcast.assert_called_once_with(
|
mock_cancel_broadcast.assert_called_once_with(
|
||||||
identifier=str(broadcast_provider_message.id),
|
identifier=str(broadcast_provider_message.id),
|
||||||
@@ -468,9 +468,13 @@ def test_send_broadcast_provider_message_errors(mocker, sample_service, provider
|
|||||||
f'app.clients.cbc_proxy.CBCProxy{provider_capitalised}.create_and_send_broadcast',
|
f'app.clients.cbc_proxy.CBCProxy{provider_capitalised}.create_and_send_broadcast',
|
||||||
side_effect=CBCProxyRetryableException('oh no'),
|
side_effect=CBCProxyRetryableException('oh no'),
|
||||||
)
|
)
|
||||||
mock_retry = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_provider_message.retry')
|
mock_retry = mocker.patch(
|
||||||
|
'app.celery.broadcast_message_tasks.send_broadcast_provider_message.retry',
|
||||||
|
side_effect=Retry
|
||||||
|
)
|
||||||
|
|
||||||
send_broadcast_provider_message(provider=provider, broadcast_event_id=str(event.id))
|
with pytest.raises(Retry):
|
||||||
|
send_broadcast_provider_message(provider=provider, broadcast_event_id=str(event.id))
|
||||||
|
|
||||||
mock_create_broadcast.assert_called_once_with(
|
mock_create_broadcast.assert_called_once_with(
|
||||||
identifier=ANY,
|
identifier=ANY,
|
||||||
@@ -493,6 +497,9 @@ def test_send_broadcast_provider_message_errors(mocker, sample_service, provider
|
|||||||
exc=mock_create_broadcast.side_effect,
|
exc=mock_create_broadcast.side_effect,
|
||||||
queue='broadcast-tasks'
|
queue='broadcast-tasks'
|
||||||
)
|
)
|
||||||
|
broadcast_provider_message = event.get_provider_message(provider)
|
||||||
|
assert broadcast_provider_message.status == BroadcastProviderMessageStatus.TECHNICAL_FAILURE
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.parametrize('num_retries, expected_countdown', [
|
@pytest.mark.parametrize('num_retries, expected_countdown', [
|
||||||
@@ -515,13 +522,17 @@ def test_send_broadcast_provider_message_delays_retry_exponentially(
|
|||||||
'app.clients.cbc_proxy.CBCProxyEE.create_and_send_broadcast',
|
'app.clients.cbc_proxy.CBCProxyEE.create_and_send_broadcast',
|
||||||
side_effect=CBCProxyRetryableException('oh no'),
|
side_effect=CBCProxyRetryableException('oh no'),
|
||||||
)
|
)
|
||||||
mock_retry = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_provider_message.retry')
|
mock_retry = mocker.patch(
|
||||||
|
'app.celery.broadcast_message_tasks.send_broadcast_provider_message.retry',
|
||||||
|
side_effect=Retry
|
||||||
|
)
|
||||||
|
|
||||||
# patch celery request context as shown here: https://stackoverflow.com/a/59870468
|
# patch celery request context as shown here: https://stackoverflow.com/a/59870468
|
||||||
mock_celery_task_request_context = mocker.patch("celery.app.task.Task.request")
|
mock_celery_task_request_context = mocker.patch("celery.app.task.Task.request")
|
||||||
mock_celery_task_request_context.retries = num_retries
|
mock_celery_task_request_context.retries = num_retries
|
||||||
|
|
||||||
send_broadcast_provider_message(provider='ee', broadcast_event_id=str(event.id))
|
with pytest.raises(Retry):
|
||||||
|
send_broadcast_provider_message(provider='ee', broadcast_event_id=str(event.id))
|
||||||
|
|
||||||
mock_create_broadcast.assert_called_once_with(
|
mock_create_broadcast.assert_called_once_with(
|
||||||
identifier=ANY,
|
identifier=ANY,
|
||||||
|
|||||||
Reference in New Issue
Block a user