Refactoring retry periods for sms high priority

app/celery/provider_tasks.py

@@ -130,7 +130,6 @@ def _deliver_sms(self, notification_id):
                 # retry with the default delay.
                 current_app.logger.exception("SMS notification delivery for id: {} failed".format(notification_id))
                 self.retry(**build_retry_task_params(notification.notification_type, notification.template.process_type))
-                # self.retry(kwargs=build_retry_task_params(notification.notification_type, notification.template.process_type))
         except self.MaxRetriesExceededError:
             message = (
                 "RETRY FAILED: Max retries reached. The task send_sms_to_provider failed for notification {}. "

app/notifications/__init__.py

@@ -3,52 +3,16 @@
 from flask import current_app
 from typing import Any, Dict
 
-# Default retry policy for all notifications.
-RETRY_POLICY_DEFAULT = {
-    "max_retries": 48,
-    "interval_start": 300,
-    "interval_step": 300,
-    "interval_max": 300,
-    "retry_errors": None,
+# Default retry periods for sending notifications.
+RETRY_DEFAULT = 300
+RETRY_HIGH = 25
+
+RETRY_PERIODS = {
+    BULK: RETRY_DEFAULT,
+    NORMAL: RETRY_DEFAULT,
+    PRIORITY: RETRY_HIGH,
 }
 
-# Retry policy for high priority notifications.
-RETRY_POLICY_HIGH = {
-    "max_retries": 48,
-    "interval_start": 25,
-    "interval_step": 25,
-    "interval_max": 25,
-    "retry_errors": None,
-}
-
-# Retry policies for each notification priority lanes.
-RETRY_POLICIES = {
-    BULK: RETRY_POLICY_DEFAULT,
-    NORMAL: RETRY_POLICY_DEFAULT,
-    PRIORITY: RETRY_POLICY_HIGH,
-}
-
-
-def build_delivery_task_params(notification_type: str, notification_process_type: str) -> Dict[str, Any]:
-    """
-    Build task params for the sending parameter tasks.
-
-    If the notification is a high priority SMS, set the retry policy to retry every 25 seconds
-    else fall back to the default retry policy of retrying every 5 minutes.
-    """
-    if current_app.config["FF_CELERY_CUSTOM_TASK_PARAMS"] is False:
-        return {}
-
-    params: dict[str, Any] = {}
-    params["retry"] = True
-    # Overring the retry policy is only supported for SMS for now;
-    # email support coming later.
-    if notification_type == SMS_TYPE:
-        params["retry_policy"] = RETRY_POLICIES[notification_process_type]
-    else:
-        params["retry_policy"] = RETRY_POLICY_DEFAULT
-    return params
-
 
 def build_retry_task_params(notification_type: str, notification_process_type: str) -> Dict[str, Any]:
     """
@@ -64,7 +28,7 @@ def build_retry_task_params(notification_type: str, notification_process_type: s
     # Overring the retry policy is only supported for SMS for now;
     # email support coming later.
     if notification_type == SMS_TYPE:
-        params["countdown"] = RETRY_POLICIES[notification_process_type]["interval_step"]
+        params["countdown"] = RETRY_PERIODS[notification_process_type]
     else:
-        params["countdown"] = RETRY_POLICY_DEFAULT["interval_step"]
+        params["countdown"] = RETRY_DEFAULT
     return params

app/notifications/process_notifications.py

@@ -35,7 +35,6 @@
     ScheduledNotification,
     Service,
 )
-from app.notifications import build_delivery_task_params
 from app.types import VerifiedNotification
 from app.utils import get_delivery_queue_for_template, get_template_instance
 from app.v2.errors import BadRequestError
@@ -214,7 +213,6 @@ def db_save_and_send_notification(notification: Notification):
         deliver_task.apply_async(
             [str(notification.id)],
             queue=notification.queue_name,
-            **build_delivery_task_params(notification.notification_type, notification.template.process_type),
         )
     except Exception:
         dao_delete_notifications_by_id(notification.id)

tests/app/notifications/test_process_notification.py

@@ -26,11 +26,7 @@
     ScheduledNotification,
     Template,
 )
-from app.notifications import (
-    RETRY_POLICY_DEFAULT,
-    RETRY_POLICY_HIGH,
-    build_delivery_task_params,
-)
+from app.notifications import RETRY_PERIODS, build_retry_task_params
 from app.notifications.process_notifications import (
     choose_queue,
     create_content_for_notification,
@@ -1062,9 +1058,7 @@ def test_db_save_and_send_notification_sends_to_queue(
 
         db_save_and_send_notification(notification=notification)
 
-        mocked.assert_called_once_with(
-            [str(notification.id)], queue=expected_queue, retry=True, retry_policy=RETRY_POLICY_DEFAULT
-        )
+        mocked.assert_called_once_with([str(notification.id)], queue=expected_queue)
 
     def test_db_save_and_send_notification_throws_exception_deletes_notification(
         self, sample_template, sample_api_key, sample_job, mocker
@@ -1101,25 +1095,22 @@ def test_db_save_and_send_notification_throws_exception_deletes_notification(
         assert NotificationHistory.query.count() == 0
 
     @pytest.mark.parametrize(
-        ("notification_type, process_type, expected_retry, expected_retry_period"),
+        ("notification_type, process_type, expected_retry_period"),
         [
-            (EMAIL_TYPE, BULK, RETRY_POLICY_DEFAULT["max_retries"], RETRY_POLICY_DEFAULT["interval_step"]),
-            (EMAIL_TYPE, NORMAL, RETRY_POLICY_DEFAULT["max_retries"], RETRY_POLICY_DEFAULT["interval_step"]),
-            (EMAIL_TYPE, PRIORITY, RETRY_POLICY_DEFAULT["max_retries"], RETRY_POLICY_DEFAULT["interval_step"]),
-            (SMS_TYPE, BULK, RETRY_POLICY_DEFAULT["max_retries"], RETRY_POLICY_DEFAULT["interval_step"]),
-            (SMS_TYPE, NORMAL, RETRY_POLICY_DEFAULT["max_retries"], RETRY_POLICY_DEFAULT["interval_step"]),
-            (SMS_TYPE, PRIORITY, RETRY_POLICY_HIGH["max_retries"], RETRY_POLICY_HIGH["interval_step"]),
+            (EMAIL_TYPE, BULK, RETRY_PERIODS[BULK]),
+            (EMAIL_TYPE, NORMAL, RETRY_PERIODS[NORMAL]),
+            (EMAIL_TYPE, PRIORITY, RETRY_PERIODS[NORMAL]),
+            (SMS_TYPE, BULK, RETRY_PERIODS[BULK]),
+            (SMS_TYPE, NORMAL, RETRY_PERIODS[NORMAL]),
+            (SMS_TYPE, PRIORITY, RETRY_PERIODS[PRIORITY]),
         ],
     )
-    def test_delivery_task_parameters(self, notification_type, process_type, expected_retry, expected_retry_period):
-        params: Dict[str, Any] = build_delivery_task_params(notification_type, process_type)
-        assert params["retry"] is True
-
-        retry_policy: Dict[str, Any] = params["retry_policy"]
-        assert retry_policy["max_retries"] == expected_retry
-        assert retry_policy["interval_start"] == expected_retry_period
-        assert retry_policy["interval_step"] == expected_retry_period
-        assert retry_policy["interval_max"] == expected_retry_period
+    def test_retry_task_parameters(self, notify_api, notification_type, process_type, expected_retry_period):
+        with notify_api.app_context():
+            params: Dict[str, Any] = build_retry_task_params(notification_type, process_type)
+
+        assert params["queue"] == QueueNames.RETRY
+        assert params["countdown"] == expected_retry_period
 
     def test_db_save_and_send_notification_throws_exception_when_missing_template(self, sample_api_key, mocker):
         mocker.patch("app.celery.provider_tasks.deliver_sms.apply_async")