From 3d1aa98b2ea344fba7fbb2373a37d4cf30f53e08 Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Wed, 22 May 2019 10:47:44 -0700 Subject: [PATCH] retention: Use a consistent ordering for processing realms. This is probably a good idea for the production use case, since then there's some consistency of behavior, and if we extend logging, one knows exactly which realms were or were not executed before a logged failure. This fixes the nondeterministic test failures we've been seeing in CI: if you use `-id` in that order_by, it happens consistently. --- zerver/lib/retention.py | 2 +- zerver/tests/test_retention.py | 10 ---------- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/zerver/lib/retention.py b/zerver/lib/retention.py index 10bb619e8b..492c4cfcb9 100644 --- a/zerver/lib/retention.py +++ b/zerver/lib/retention.py @@ -131,7 +131,7 @@ def clean_unused_messages() -> None: def archive_messages() -> None: - for realm in Realm.objects.filter(message_retention_days__isnull=False): + for realm in Realm.objects.filter(message_retention_days__isnull=False).order_by("id"): move_expired_messages_to_archive(realm) move_expired_user_messages_to_archive(realm) move_expired_attachments_to_archive(realm) diff --git a/zerver/tests/test_retention.py b/zerver/tests/test_retention.py index e0125c3d72..a15c7b80d0 100644 --- a/zerver/tests/test_retention.py +++ b/zerver/tests/test_retention.py @@ -263,20 +263,10 @@ class TestRetentionLib(ZulipTestCase): # Get expired user messages by message ids expected_user_msgs_ids = list(UserMessage.objects.filter( message_id__in=expected_message_ids).order_by('id').values_list('id', flat=True)) - user_messages = list(UserMessage.objects.select_related( - "message").filter(message_id__in=expected_message_ids).order_by('message_id')) msgs_qty = Message.objects.count() archive_messages() - # Temporary debugging code while we investigate CI failures - for user_message in user_messages: # nocoverage - archived = ArchivedUserMessage.objects.filter( - message_id=user_message.message_id, - user_profile_id=user_message.user_profile_id).exists() - if not archived: - print("Missing:", user_message, user_message.message_id, flush=True) - # Compare archived messages and user messages with expired messages self.assertEqual(ArchivedMessage.objects.count(), len(expected_message_ids)) self.assertEqual(ArchivedUserMessage.objects.count(), len(expected_user_msgs_ids))