change countdown from 30 to 60 seconds for message sends to better match batch insert timing

This commit is contained in:
Kenneth Kehl
2025-01-13 13:35:40 -08:00
parent eac21788a1
commit 4965bc2354
12 changed files with 29 additions and 29 deletions

View File

@@ -324,7 +324,7 @@ def batch_insert_notifications(self):
current_app.logger.exception("Notification batch insert failed") current_app.logger.exception("Notification batch insert failed")
for n in batch: for n in batch:
# Use 'created_at' as a TTL so we don't retry infinitely # Use 'created_at' as a TTL so we don't retry infinitely
if datetime.fromisoformat(n.created_at) < utc_now() - timedelta(minutes=1): if datetime.fromisoformat(n.created_at) < utc_now() - timedelta(seconds=50):
current_app.logger.warning( current_app.logger.warning(
f"Abandoning stale data, could not write to db: {n.serialize_for_redis(n)}" f"Abandoning stale data, could not write to db: {n.serialize_for_redis(n)}"
) )

View File

@@ -256,7 +256,7 @@ def save_sms(self, service_id, notification_id, encrypted_notification, sender_i
) )
) )
provider_tasks.deliver_sms.apply_async( provider_tasks.deliver_sms.apply_async(
[str(saved_notification.id)], queue=QueueNames.SEND_SMS, countdown=30 [str(saved_notification.id)], queue=QueueNames.SEND_SMS, countdown=60
) )
current_app.logger.debug( current_app.logger.debug(

View File

@@ -174,7 +174,7 @@ def send_notification_to_queue_detached(
deliver_task = provider_tasks.deliver_email deliver_task = provider_tasks.deliver_email
try: try:
deliver_task.apply_async([str(notification_id)], queue=queue, countdown=30) deliver_task.apply_async([str(notification_id)], queue=queue, countdown=60)
except Exception: except Exception:
dao_delete_notifications_by_id(notification_id) dao_delete_notifications_by_id(notification_id)
raise raise

View File

@@ -311,10 +311,10 @@ def test_replay_created_notifications(notify_db_session, sample_service, mocker)
replay_created_notifications() replay_created_notifications()
email_delivery_queue.assert_called_once_with( email_delivery_queue.assert_called_once_with(
[str(old_email.id)], queue="send-email-tasks", countdown=30 [str(old_email.id)], queue="send-email-tasks", countdown=60
) )
sms_delivery_queue.assert_called_once_with( sms_delivery_queue.assert_called_once_with(
[str(old_sms.id)], queue="send-sms-tasks", countdown=30 [str(old_sms.id)], queue="send-sms-tasks", countdown=60
) )

View File

@@ -434,7 +434,7 @@ def test_should_send_template_to_correct_sms_task_and_persist(
assert persisted_notification.personalisation == {} assert persisted_notification.personalisation == {}
assert persisted_notification.notification_type == NotificationType.SMS assert persisted_notification.notification_type == NotificationType.SMS
mocked_deliver_sms.assert_called_once_with( mocked_deliver_sms.assert_called_once_with(
[str(persisted_notification.id)], queue="send-sms-tasks", countdown=30 [str(persisted_notification.id)], queue="send-sms-tasks", countdown=60
) )
@@ -470,7 +470,7 @@ def test_should_save_sms_if_restricted_service_and_valid_number(
assert not persisted_notification.personalisation assert not persisted_notification.personalisation
assert persisted_notification.notification_type == NotificationType.SMS assert persisted_notification.notification_type == NotificationType.SMS
provider_tasks.deliver_sms.apply_async.assert_called_once_with( provider_tasks.deliver_sms.apply_async.assert_called_once_with(
[str(persisted_notification.id)], queue="send-sms-tasks", countdown=30 [str(persisted_notification.id)], queue="send-sms-tasks", countdown=60
) )
@@ -598,7 +598,7 @@ def test_should_save_sms_template_to_and_persist_with_job_id(sample_job, mocker)
assert persisted_notification.notification_type == NotificationType.SMS assert persisted_notification.notification_type == NotificationType.SMS
provider_tasks.deliver_sms.apply_async.assert_called_once_with( provider_tasks.deliver_sms.apply_async.assert_called_once_with(
[str(persisted_notification.id)], queue="send-sms-tasks", countdown=30 [str(persisted_notification.id)], queue="send-sms-tasks", countdown=60
) )

View File

@@ -264,7 +264,7 @@ def test_send_notification_to_queue(
send_notification_to_queue(notification=notification, queue=requested_queue) send_notification_to_queue(notification=notification, queue=requested_queue)
mocked.assert_called_once_with( mocked.assert_called_once_with(
[str(notification.id)], queue=expected_queue, countdown=30 [str(notification.id)], queue=expected_queue, countdown=60
) )
@@ -278,7 +278,7 @@ def test_send_notification_to_queue_throws_exception_deletes_notification(
with pytest.raises(Boto3Error): with pytest.raises(Boto3Error):
send_notification_to_queue(sample_notification, False) send_notification_to_queue(sample_notification, False)
mocked.assert_called_once_with( mocked.assert_called_once_with(
[(str(sample_notification.id))], queue="send-sms-tasks", countdown=30 [(str(sample_notification.id))], queue="send-sms-tasks", countdown=60
) )
assert _get_notification_query_count() == 0 assert _get_notification_query_count() == 0

View File

@@ -73,7 +73,7 @@ def test_create_invited_org_user(
# assert len(notification.personalisation["url"]) > len(expected_start_of_invite_url) # assert len(notification.personalisation["url"]) > len(expected_start_of_invite_url)
mocked.assert_called_once_with( mocked.assert_called_once_with(
[(str(notification.id))], queue="notify-internal-tasks", countdown=30 [(str(notification.id))], queue="notify-internal-tasks", countdown=60
) )

View File

@@ -151,7 +151,7 @@ def test_send_notification_with_placeholders_replaced(
) )
mocked.assert_called_once_with( mocked.assert_called_once_with(
[notification_id], queue="send-email-tasks", countdown=30 [notification_id], queue="send-email-tasks", countdown=60
) )
assert response.status_code == 201 assert response.status_code == 201
assert response_data["body"] == "Hello Jo\nThis is an email from GOV.UK" assert response_data["body"] == "Hello Jo\nThis is an email from GOV.UK"
@@ -423,7 +423,7 @@ def test_should_allow_valid_sms_notification(notify_api, sample_template, mocker
notification_id = response_data["notification"]["id"] notification_id = response_data["notification"]["id"]
mocked.assert_called_once_with( mocked.assert_called_once_with(
[notification_id], queue="send-sms-tasks", countdown=30 [notification_id], queue="send-sms-tasks", countdown=60
) )
assert response.status_code == 201 assert response.status_code == 201
assert notification_id assert notification_id
@@ -480,7 +480,7 @@ def test_should_allow_valid_email_notification(
response_data = json.loads(response.get_data(as_text=True))["data"] response_data = json.loads(response.get_data(as_text=True))["data"]
notification_id = response_data["notification"]["id"] notification_id = response_data["notification"]["id"]
app.celery.provider_tasks.deliver_email.apply_async.assert_called_once_with( app.celery.provider_tasks.deliver_email.apply_async.assert_called_once_with(
[notification_id], queue="send-email-tasks", countdown=30 [notification_id], queue="send-email-tasks", countdown=60
) )
assert response.status_code == 201 assert response.status_code == 201
@@ -624,7 +624,7 @@ def test_should_send_email_if_team_api_key_and_a_service_user(
) )
app.celery.provider_tasks.deliver_email.apply_async.assert_called_once_with( app.celery.provider_tasks.deliver_email.apply_async.assert_called_once_with(
[fake_uuid], queue="send-email-tasks", countdown=30 [fake_uuid], queue="send-email-tasks", countdown=60
) )
assert response.status_code == 201 assert response.status_code == 201
@@ -662,7 +662,7 @@ def test_should_send_sms_to_anyone_with_test_key(
], ],
) )
app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with( app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with(
[fake_uuid], queue="send-sms-tasks", countdown=30 [fake_uuid], queue="send-sms-tasks", countdown=60
) )
assert response.status_code == 201 assert response.status_code == 201
@@ -701,7 +701,7 @@ def test_should_send_email_to_anyone_with_test_key(
) )
app.celery.provider_tasks.deliver_email.apply_async.assert_called_once_with( app.celery.provider_tasks.deliver_email.apply_async.assert_called_once_with(
[fake_uuid], queue="send-email-tasks", countdown=30 [fake_uuid], queue="send-email-tasks", countdown=60
) )
assert response.status_code == 201 assert response.status_code == 201
@@ -739,7 +739,7 @@ def test_should_send_sms_if_team_api_key_and_a_service_user(
) )
app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with( app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with(
[fake_uuid], queue="send-sms-tasks", countdown=30 [fake_uuid], queue="send-sms-tasks", countdown=60
) )
assert response.status_code == 201 assert response.status_code == 201
@@ -796,7 +796,7 @@ def test_should_persist_notification(
], ],
) )
mocked.assert_called_once_with([fake_uuid], queue=queue_name, countdown=30) mocked.assert_called_once_with([fake_uuid], queue=queue_name, countdown=60)
assert response.status_code == 201 assert response.status_code == 201
notification = notifications_dao.get_notification_by_id(fake_uuid) notification = notifications_dao.get_notification_by_id(fake_uuid)
@@ -857,7 +857,7 @@ def test_should_delete_notification_and_return_error_if_redis_fails(
) )
assert str(e.value) == "failed to talk to redis" assert str(e.value) == "failed to talk to redis"
mocked.assert_called_once_with([fake_uuid], queue=queue_name, countdown=30) mocked.assert_called_once_with([fake_uuid], queue=queue_name, countdown=60)
assert not notifications_dao.get_notification_by_id(fake_uuid) assert not notifications_dao.get_notification_by_id(fake_uuid)
assert not NotificationHistory.query.get(fake_uuid) assert not NotificationHistory.query.get(fake_uuid)
@@ -1190,7 +1190,7 @@ def test_should_allow_store_original_number_on_sms_notification(
notification_id = response_data["notification"]["id"] notification_id = response_data["notification"]["id"]
mocked.assert_called_once_with( mocked.assert_called_once_with(
[notification_id], queue="send-sms-tasks", countdown=30 [notification_id], queue="send-sms-tasks", countdown=60
) )
assert response.status_code == 201 assert response.status_code == 201
assert notification_id assert notification_id

View File

@@ -3025,7 +3025,7 @@ def test_verify_reply_to_email_address_should_send_verification_email(
assert notification.template_id == verify_reply_to_address_email_template.id assert notification.template_id == verify_reply_to_address_email_template.id
assert response["data"] == {"id": str(notification.id)} assert response["data"] == {"id": str(notification.id)}
mocked.assert_called_once_with( mocked.assert_called_once_with(
[str(notification.id)], queue="notify-internal-tasks", countdown=30 [str(notification.id)], queue="notify-internal-tasks", countdown=60
) )
assert ( assert (
notification.reply_to_text notification.reply_to_text

View File

@@ -90,7 +90,7 @@ def test_create_invited_user(
) )
mocked.assert_called_once_with( mocked.assert_called_once_with(
[(str(notification.id))], queue="notify-internal-tasks", countdown=30 [(str(notification.id))], queue="notify-internal-tasks", countdown=60
) )

View File

@@ -664,7 +664,7 @@ def test_send_already_registered_email(
stmt = select(Notification) stmt = select(Notification)
notification = db.session.execute(stmt).scalars().first() notification = db.session.execute(stmt).scalars().first()
mocked.assert_called_once_with( mocked.assert_called_once_with(
([str(notification.id)]), queue="notify-internal-tasks", countdown=30 ([str(notification.id)]), queue="notify-internal-tasks", countdown=60
) )
assert ( assert (
notification.reply_to_text notification.reply_to_text
@@ -703,7 +703,7 @@ def test_send_user_confirm_new_email_returns_204(
stmt = select(Notification) stmt = select(Notification)
notification = db.session.execute(stmt).scalars().first() notification = db.session.execute(stmt).scalars().first()
mocked.assert_called_once_with( mocked.assert_called_once_with(
([str(notification.id)]), queue="notify-internal-tasks", countdown=30 ([str(notification.id)]), queue="notify-internal-tasks", countdown=60
) )
assert ( assert (
notification.reply_to_text notification.reply_to_text

View File

@@ -231,7 +231,7 @@ def test_send_user_sms_code(client, sample_user, sms_code_template, mocker):
assert notification.reply_to_text == notify_service.get_default_sms_sender() assert notification.reply_to_text == notify_service.get_default_sms_sender()
app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with( app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with(
([str(notification.id)]), queue="notify-internal-tasks", countdown=30 ([str(notification.id)]), queue="notify-internal-tasks", countdown=60
) )
@@ -267,7 +267,7 @@ def test_send_user_code_for_sms_with_optional_to_field(
notification = Notification.query.first() notification = Notification.query.first()
assert notification.to == "1" assert notification.to == "1"
app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with( app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with(
([str(notification.id)]), queue="notify-internal-tasks", countdown=30 ([str(notification.id)]), queue="notify-internal-tasks", countdown=60
) )
@@ -349,7 +349,7 @@ def test_send_new_user_email_verification(
notification = Notification.query.first() notification = Notification.query.first()
assert _get_verify_code_count() == 0 assert _get_verify_code_count() == 0
mocked.assert_called_once_with( mocked.assert_called_once_with(
([str(notification.id)]), queue="notify-internal-tasks", countdown=30 ([str(notification.id)]), queue="notify-internal-tasks", countdown=60
) )
assert ( assert (
notification.reply_to_text notification.reply_to_text
@@ -495,7 +495,7 @@ def test_send_user_email_code(
assert noti.to == "1" assert noti.to == "1"
assert str(noti.template_id) == current_app.config["EMAIL_2FA_TEMPLATE_ID"] assert str(noti.template_id) == current_app.config["EMAIL_2FA_TEMPLATE_ID"]
deliver_email.assert_called_once_with( deliver_email.assert_called_once_with(
[str(noti.id)], queue="notify-internal-tasks", countdown=30 [str(noti.id)], queue="notify-internal-tasks", countdown=60
) )