From 9e4f53e29963a9f22908affdfd40285af211972d Mon Sep 17 00:00:00 2001 From: James Kiger Date: Wed, 24 Apr 2024 08:56:34 -0400 Subject: [PATCH 1/4] Add remove_old_import_tasks and test --- kpi/maintenance_tasks.py | 15 ++++++++++++++- kpi/tasks.py | 3 ++- kpi/tests/test_import_tasks.py | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 kpi/tests/test_import_tasks.py diff --git a/kpi/maintenance_tasks.py b/kpi/maintenance_tasks.py index 43d4bc8313..b87522b86a 100644 --- a/kpi/maintenance_tasks.py +++ b/kpi/maintenance_tasks.py @@ -4,7 +4,7 @@ from django.db.models import Exists, OuterRef, Q from django.utils import timezone -from kpi.models import AssetSnapshot +from kpi.models import AssetSnapshot, ImportTask def remove_old_asset_snapshots(): @@ -26,3 +26,16 @@ def remove_old_asset_snapshots(): ).delete() if not count: break + + +def remove_old_import_tasks(): + delete_queryset = ImportTask.objects.filter( + date_created__lt=timezone.now() - timedelta(days=90), + ) + + while True: + count, _ = delete_queryset.filter( + pk__in=delete_queryset[:1000] + ).delete() + if not count: + break diff --git a/kpi/tasks.py b/kpi/tasks.py index 553b71d3d6..f4e2ec700e 100644 --- a/kpi/tasks.py +++ b/kpi/tasks.py @@ -9,7 +9,7 @@ from kobo.apps.markdownx_uploader.tasks import remove_unused_markdown_files from kobo.celery import celery_app from kpi.constants import LIMIT_HOURS_23 -from kpi.maintenance_tasks import remove_old_asset_snapshots +from kpi.maintenance_tasks import remove_old_asset_snapshots, remove_old_import_tasks from kpi.models.asset import Asset from kpi.models.import_export_task import ( ExportTask, @@ -102,4 +102,5 @@ def perform_maintenance(): Run daily maintenance tasks """ remove_unused_markdown_files() + remove_old_import_tasks() remove_old_asset_snapshots() diff --git a/kpi/tests/test_import_tasks.py b/kpi/tests/test_import_tasks.py new file mode 100644 index 0000000000..a874c9619f --- /dev/null +++ b/kpi/tests/test_import_tasks.py @@ -0,0 +1,33 @@ +# coding: utf-8 +from datetime import timedelta + +from django.contrib.auth.models import User +from django.utils import timezone + +from kpi.maintenance_tasks import remove_old_import_tasks +from kpi.models import ImportTask +from kpi.tests.base_test_case import BaseTestCase + + +class AssetImportTaskHousekeepingTest(BaseTestCase): + fixtures = ['test_data'] + + def setUp(self): + self.user = User.objects.get(username='someuser') + + def test_remove_old_import_tasks(self): + old_task = ImportTask.objects.create( + user=self.user, + data='{}', + date_created=timezone.now() - timedelta(days=92), + ) + + old_task.date_created = timezone.now() - timedelta(days=95) + old_task.save(update_fields=['date_created']) + + new_task = ImportTask.objects.create(user=self.user, data='{}') + + remove_old_import_tasks() + + assert ImportTask.objects.filter(id=new_task.id).exists() + assert not ImportTask.objects.filter(id=old_task.id).exists() From 3a1020bbfa2b779d5e7b37a25d683327864d1de3 Mon Sep 17 00:00:00 2001 From: James Kiger Date: Wed, 24 Apr 2024 09:05:17 -0400 Subject: [PATCH 2/4] Remove unnecessary line --- kpi/tests/test_import_tasks.py | 1 - 1 file changed, 1 deletion(-) diff --git a/kpi/tests/test_import_tasks.py b/kpi/tests/test_import_tasks.py index a874c9619f..a7a6c63220 100644 --- a/kpi/tests/test_import_tasks.py +++ b/kpi/tests/test_import_tasks.py @@ -19,7 +19,6 @@ def test_remove_old_import_tasks(self): old_task = ImportTask.objects.create( user=self.user, data='{}', - date_created=timezone.now() - timedelta(days=92), ) old_task.date_created = timezone.now() - timedelta(days=95) From 26f26b4fed2c2c09018e869c449da68c088dbaf7 Mon Sep 17 00:00:00 2001 From: James Kiger Date: Wed, 24 Apr 2024 10:58:46 -0400 Subject: [PATCH 3/4] Refactor chunked deletion and add test note --- kpi/maintenance_tasks.py | 15 +++------------ kpi/tests/test_import_tasks.py | 2 +- kpi/utils/chunked_delete.py | 8 ++++++++ 3 files changed, 12 insertions(+), 13 deletions(-) create mode 100644 kpi/utils/chunked_delete.py diff --git a/kpi/maintenance_tasks.py b/kpi/maintenance_tasks.py index b87522b86a..e73a2e6e4b 100644 --- a/kpi/maintenance_tasks.py +++ b/kpi/maintenance_tasks.py @@ -5,6 +5,7 @@ from django.utils import timezone from kpi.models import AssetSnapshot, ImportTask +from kpi.utils.chunked_delete import chunked_delete def remove_old_asset_snapshots(): @@ -20,12 +21,7 @@ def remove_old_asset_snapshots(): date_created__lt=timezone.now() - timedelta(days=days), ).filter(Exists(newer_snapshot_for_asset) | Q(asset_version=None)) - while True: - count, _ = delete_queryset.filter( - pk__in=delete_queryset[:1000] - ).delete() - if not count: - break + chunked_delete(delete_queryset) def remove_old_import_tasks(): @@ -33,9 +29,4 @@ def remove_old_import_tasks(): date_created__lt=timezone.now() - timedelta(days=90), ) - while True: - count, _ = delete_queryset.filter( - pk__in=delete_queryset[:1000] - ).delete() - if not count: - break + chunked_delete(delete_queryset) diff --git a/kpi/tests/test_import_tasks.py b/kpi/tests/test_import_tasks.py index a7a6c63220..efc4225f6d 100644 --- a/kpi/tests/test_import_tasks.py +++ b/kpi/tests/test_import_tasks.py @@ -20,7 +20,7 @@ def test_remove_old_import_tasks(self): user=self.user, data='{}', ) - + # Because of `auto_date_now`, we cannot specify created_date on creation old_task.date_created = timezone.now() - timedelta(days=95) old_task.save(update_fields=['date_created']) diff --git a/kpi/utils/chunked_delete.py b/kpi/utils/chunked_delete.py new file mode 100644 index 0000000000..64ae01e123 --- /dev/null +++ b/kpi/utils/chunked_delete.py @@ -0,0 +1,8 @@ +from django.db.models import QuerySet + + +def chunked_delete(queryset: QuerySet): + while True: + count, _ = queryset.filter(pk__in=queryset[:1000]).delete() + if not count: + break From b1d0b2b6d2abecf2a1387ab48919be7e175937b6 Mon Sep 17 00:00:00 2001 From: James Kiger Date: Wed, 24 Apr 2024 11:45:58 -0400 Subject: [PATCH 4/4] Add constance config for import cleanup and fix user model in test --- kobo/settings/base.py | 10 +++++++++- kpi/maintenance_tasks.py | 4 +++- kpi/tests/test_import_tasks.py | 3 ++- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/kobo/settings/base.py b/kobo/settings/base.py index ad6b8728ba..06ec6b5005 100644 --- a/kobo/settings/base.py +++ b/kobo/settings/base.py @@ -363,6 +363,11 @@ 'Number of days to keep asset snapshots', 'positive_int' ), + 'IMPORT_TASK_DAYS_RETENTION': ( + 90, + 'Number of days to keep import tasks', + 'positive_int', + ), 'FREE_TIER_THRESHOLDS': ( LazyJSONSerializable(FREE_TIER_NO_THRESHOLDS), 'Free tier thresholds: storage in kilobytes, ' @@ -661,10 +666,13 @@ 'PROJECT_OWNERSHIP_AUTO_ACCEPT_INVITES', ), 'Trash bin': ( - 'ASSET_SNAPSHOT_DAYS_RETENTION', 'ACCOUNT_TRASH_GRACE_PERIOD', 'PROJECT_TRASH_GRACE_PERIOD', ), + 'Regular maintenance settings': ( + 'ASSET_SNAPSHOT_DAYS_RETENTION', + 'IMPORT_TASK_DAYS_RETENTION', + ), 'Tier settings': ( 'FREE_TIER_THRESHOLDS', 'FREE_TIER_DISPLAY', diff --git a/kpi/maintenance_tasks.py b/kpi/maintenance_tasks.py index e73a2e6e4b..4ddcdabc3d 100644 --- a/kpi/maintenance_tasks.py +++ b/kpi/maintenance_tasks.py @@ -25,8 +25,10 @@ def remove_old_asset_snapshots(): def remove_old_import_tasks(): + days = constance.config.IMPORT_TASK_DAYS_RETENTION + delete_queryset = ImportTask.objects.filter( - date_created__lt=timezone.now() - timedelta(days=90), + date_created__lt=timezone.now() - timedelta(days=days), ) chunked_delete(delete_queryset) diff --git a/kpi/tests/test_import_tasks.py b/kpi/tests/test_import_tasks.py index efc4225f6d..6914f650f7 100644 --- a/kpi/tests/test_import_tasks.py +++ b/kpi/tests/test_import_tasks.py @@ -1,7 +1,7 @@ # coding: utf-8 from datetime import timedelta -from django.contrib.auth.models import User +from django.contrib.auth import get_user_model from django.utils import timezone from kpi.maintenance_tasks import remove_old_import_tasks @@ -13,6 +13,7 @@ class AssetImportTaskHousekeepingTest(BaseTestCase): fixtures = ['test_data'] def setUp(self): + User = get_user_model() self.user = User.objects.get(username='someuser') def test_remove_old_import_tasks(self):