Skip to content

Commit

Permalink
review and cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
Chris Johnson committed Jan 22, 2025
1 parent c6b5c49 commit 6aa055d
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 14 deletions.
17 changes: 3 additions & 14 deletions app/celery/process_delivery_status_result_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
CARRIER_SMS_MAX_RETRIES,
CARRIER_SMS_MAX_RETRY_WINDOW,
DELIVERY_STATUS_CALLBACK_TYPE,
NOTIFICATION_CREATED,
NOTIFICATION_DELIVERED,
NOTIFICATION_PENDING,
NOTIFICATION_PERMANENT_FAILURE,
Expand All @@ -27,7 +26,6 @@
)
from app.dao.notifications_dao import (
dao_get_notification_by_reference,
dao_update_notification,
dao_update_sms_notification_delivery_status,
dao_update_sms_notification_status_to_created_for_retry,
)
Expand Down Expand Up @@ -207,7 +205,6 @@ def sms_status_update(
sms_status: SmsStatusRecord,
event_timestamp: str | None = None,
event_in_seconds: int = 300, # Don't retry by default
notification: Notification = None,
) -> None:
"""Get and update a notification.
Expand All @@ -219,8 +216,7 @@ def sms_status_update(
Raises:
NonRetryableException: Unable to update the notification
"""
if notification is None:
notification = _get_notification(sms_status.reference, sms_status.provider, event_timestamp, event_in_seconds)
notification = _get_notification(sms_status.reference, sms_status.provider, event_timestamp, event_in_seconds)
last_updated_at = notification.updated_at

current_app.logger.info(
Expand Down Expand Up @@ -370,17 +366,10 @@ def update_sms_retry_count(notification_retry_id: str, initial_value: int = 0, t
)


def update_notification_to_created(
notification,
):
notification.status = NOTIFICATION_CREATED
dao_update_notification(notification)


def sms_attempt_retry(
sms_status: SmsStatusRecord,
event_timestamp: str | None = None,
event_in_seconds: int = 300, # Don't retry by default
event_in_seconds: int = 300, # Don't retry _get_notification by default
):
"""Attempt retry sending notification.
Expand All @@ -390,7 +379,7 @@ def sms_attempt_retry(
Args:
sms_status (SmsStatusRecord): The status record update
event_timestamp (str | None, optional): Timestamp the Pinpoint event came in. Defaults to None.
event_in_seconds (int, optional): How many seconds Twilio updates have retried. Defaults to 300
event_in_seconds (int, optional): How long since initial event. Defaults to 300
Raises:
NonRetryableException: Unable to update SMS retry count or update the notification
Expand Down
15 changes: 15 additions & 0 deletions tests/app/celery/test_process_delivery_status_result_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -725,3 +725,18 @@ def test_sms_attempt_retry_notification_set_to_permanent_failure_when_retry_cond
sms_attempt_retry(sms_status)
updated_notification = get_notification_by_id(notification.id)
assert updated_notification.status == NOTIFICATION_PERMANENT_FAILURE


def test_sms_attempt_retry_check_and_queue_exception(mocker, sample_notification):
notification = sample_notification()
sms_status = SmsStatusRecord(
None, notification.reference, NOTIFICATION_TEMPORARY_FAILURE, STATUS_REASON_RETRYABLE, PINPOINT_PROVIDER
)
mock_logger = mocker.patch('app.celery.process_delivery_status_result_tasks.current_app.logger.exception')
mocker.patch('app.celery.process_delivery_status_result_tasks.update_sms_retry_count')
mocker.patch('app.celery.process_delivery_status_result_tasks.can_retry_sms_request', return_value=False)
mocker.patch('app.celery.process_delivery_status_result_tasks.check_and_queue_callback_task', side_effect=Exception)

# Exception is caught and not re-raised
sms_status_update(sms_status)
mock_logger.assert_called_once_with('Failed to check_and_queue_callback_task for notification: %s', notification.id)

0 comments on commit 6aa055d

Please sign in to comment.