From c8ebb365d4b058bce0265253afe295f8e3b0836c Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Mon, 13 Dec 2021 16:59:25 +0000 Subject: [PATCH] Make limit of DAO timeout function more obvious We're going to iterate how we use the function with a limit, so we shouldn't say it's "temporary" anymore. We don't need to change the default, but having it in the function parameters makes it easier to see the funtion doesn't time out all notifications, just some. --- app/dao/notifications_dao.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index acfa286e3..58d7dff47 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -489,7 +489,7 @@ def dao_delete_notifications_by_id(notification_id): ).delete(synchronize_session='fetch') -def dao_timeout_notifications(cutoff_time): +def dao_timeout_notifications(cutoff_time, limit=100000): """ Set email and SMS notifications (only) to "temporary-failure" status if they're still sending from before the specified cutoff_time. @@ -498,15 +498,11 @@ def dao_timeout_notifications(cutoff_time): current_statuses = [NOTIFICATION_SENDING, NOTIFICATION_PENDING] new_status = NOTIFICATION_TEMPORARY_FAILURE - # TEMPORARY: limit the notifications to 100K as otherwise we - # see an issues where the task vanishes after it starts executing - # - we believe this is a OOM error but there are no logs. From - # experimentation we've found we can safely process up to 100K. notifications = Notification.query.filter( Notification.created_at < cutoff_time, Notification.status.in_(current_statuses), Notification.notification_type.in_([SMS_TYPE, EMAIL_TYPE]) - ).limit(100000).all() + ).limit(limit).all() Notification.query.filter( Notification.created_at < cutoff_time,