mirror of
https://github.com/GSA/notifications-api.git
synced 2026-08-25 00:33:41 -04:00
Remove the hourly loop and replace it with a while loop, which check if we still have things to delete for the day.
We are already limiting the query by a query limit.
This commit is contained in:
@@ -311,11 +311,9 @@ def delete_notifications_older_than_retention_by_type(notification_type, qry_lim
|
|||||||
|
|
||||||
day_to_delete_backwards_from = get_london_midnight_in_utc(
|
day_to_delete_backwards_from = get_london_midnight_in_utc(
|
||||||
convert_utc_to_bst(datetime.utcnow()).date()) - timedelta(days=f.days_of_retention)
|
convert_utc_to_bst(datetime.utcnow()).date()) - timedelta(days=f.days_of_retention)
|
||||||
delete_count_per_call = 1
|
|
||||||
while delete_count_per_call > 0:
|
deleted += _move_notifications_to_notification_history(
|
||||||
delete_count_per_call = _move_notifications_to_notification_history(
|
notification_type, f.service_id, day_to_delete_backwards_from, qry_limit)
|
||||||
notification_type, f.service_id, day_to_delete_backwards_from, qry_limit)
|
|
||||||
deleted += delete_count_per_call
|
|
||||||
|
|
||||||
current_app.logger.info(
|
current_app.logger.info(
|
||||||
'Deleting {} notifications for services without flexible data retention'.format(notification_type))
|
'Deleting {} notifications for services without flexible data retention'.format(notification_type))
|
||||||
@@ -399,23 +397,15 @@ def _move_notifications_to_notification_history(notification_type, service_id, d
|
|||||||
_delete_letters_from_s3(
|
_delete_letters_from_s3(
|
||||||
notification_type, service_id, day_to_delete_backwards_from, qry_limit
|
notification_type, service_id, day_to_delete_backwards_from, qry_limit
|
||||||
)
|
)
|
||||||
|
delete_count_per_call = 1
|
||||||
stop = -1 # exclusive, we want to include 0
|
while delete_count_per_call > 0:
|
||||||
step = -1
|
delete_count_per_call = insert_notification_history_delete_notifications(
|
||||||
for hour_delta in range(23, stop, step):
|
|
||||||
# We find the timestamp we want to delete all notifications backwards from
|
|
||||||
# We then start 23 hours ago, and do an insert notification history before deleting all notifications older
|
|
||||||
# We then look 22 hours ago, do an insert notifications history before deleting all notifications older
|
|
||||||
# We continue this until we reach the original timestamp we wanted to delete notifications backwardsfrom
|
|
||||||
# This enables us to break this into smaller database queries
|
|
||||||
timestamp_to_delete_backwards_from = day_to_delete_backwards_from - timedelta(hours=hour_delta)
|
|
||||||
|
|
||||||
deleted += insert_notification_history_delete_notifications(
|
|
||||||
notification_type=notification_type,
|
notification_type=notification_type,
|
||||||
service_id=service_id,
|
service_id=service_id,
|
||||||
timestamp_to_delete_backwards_from=timestamp_to_delete_backwards_from,
|
timestamp_to_delete_backwards_from=day_to_delete_backwards_from,
|
||||||
qry_limit=qry_limit
|
qry_limit=qry_limit
|
||||||
)
|
)
|
||||||
|
deleted += delete_count_per_call
|
||||||
|
|
||||||
# Deleting test Notifications, test notifications are not persisted to NotificationHistory
|
# Deleting test Notifications, test notifications are not persisted to NotificationHistory
|
||||||
Notification.query.filter(
|
Notification.query.filter(
|
||||||
|
|||||||
@@ -222,10 +222,14 @@ def test_delete_notifications_does_try_to_delete_from_s3_when_letter_has_not_bee
|
|||||||
mock_get_s3.assert_not_called()
|
mock_get_s3.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
|
@freeze_time('2020-03-25 00:01')
|
||||||
def test_delete_notifications_calls_subquery_multiple_times(sample_template):
|
def test_delete_notifications_calls_subquery_multiple_times(sample_template):
|
||||||
create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=8), status='delivered')
|
create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=7, minutes=3),
|
||||||
create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=8), status='delivered')
|
status='delivered')
|
||||||
create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=8), status='delivered')
|
create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=7, minutes=3),
|
||||||
|
status='delivered')
|
||||||
|
create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=7, minutes=3),
|
||||||
|
status='delivered')
|
||||||
|
|
||||||
assert Notification.query.count() == 3
|
assert Notification.query.count() == 3
|
||||||
delete_notifications_older_than_retention_by_type('sms', qry_limit=1)
|
delete_notifications_older_than_retention_by_type('sms', qry_limit=1)
|
||||||
|
|||||||
Reference in New Issue
Block a user