From 7c7cb67fb818b7e37dbb8e7ad52ec8ce6f2faada Mon Sep 17 00:00:00 2001 From: Harsh Bhikadia Date: Tue, 12 Oct 2021 00:01:31 +0530 Subject: [PATCH] feat: raw delete expired instead of `Queryset.delete` (#235) * feat: raw delete expired instead of `Queryset.delete` This is avoid unncessary loading of objects in-memory in certain conditions * minor: verbose fix * minor: verbose fix * minor: verbose fix * minor: verbose simplified * test: unit test for `utils.raw_delete` added * minor: fix some lint issues * minor: fix some lint issues * minor: fix method names * [test] `MigrationTests`: calling `makemigrations --check` instead of calling internal logic of `makemigrations` to check if no pending migrations * [test] import optimized * [test] fix - wrong function name --- django_celery_results/managers.py | 4 +-- django_celery_results/utils.py | 5 ++++ t/unit/test_migrations.py | 39 +++++++++++------------------ t/unit/test_utils.py | 41 +++++++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 27 deletions(-) create mode 100644 t/unit/test_utils.py diff --git a/django_celery_results/managers.py b/django_celery_results/managers.py index 7e6a5f22..3c0789f5 100644 --- a/django_celery_results/managers.py +++ b/django_celery_results/managers.py @@ -8,7 +8,7 @@ from django.conf import settings from django.db import connections, models, router, transaction -from .utils import now +from .utils import now, raw_delete W_ISOLATION_REP = """ Polling results with transaction isolation level 'repeatable-read' @@ -88,7 +88,7 @@ def get_all_expired(self, expires): def delete_expired(self, expires): """Delete all expired results.""" with transaction.atomic(): - self.get_all_expired(expires).delete() + raw_delete(queryset=self.get_all_expired(expires)) class TaskResultManager(ResultManager): diff --git a/django_celery_results/utils.py b/django_celery_results/utils.py index 32fcaa67..8a4b2851 100644 --- a/django_celery_results/utils.py +++ b/django_celery_results/utils.py @@ -15,3 +15,8 @@ def now(): return now_localtime(timezone.now()) else: return timezone.now() + + +def raw_delete(queryset): + """Raw delete given queryset.""" + return queryset._raw_delete(queryset.db) diff --git a/t/unit/test_migrations.py b/t/unit/test_migrations.py index fd3fcc24..a07d2edb 100644 --- a/t/unit/test_migrations.py +++ b/t/unit/test_migrations.py @@ -1,11 +1,7 @@ import os -from django.apps import apps -from django.db.migrations.autodetector import MigrationAutodetector -from django.db.migrations.loader import MigrationLoader -from django.db.migrations.questioner import NonInteractiveMigrationQuestioner -from django.db.migrations.state import ProjectState -from django.test import TestCase +from django.core.management import call_command +from django.test import TestCase, override_settings from django_celery_results import migrations as result_migrations @@ -24,27 +20,20 @@ def test_no_duplicate_migration_numbers(self): msg='Detected migration files with the same migration number') def test_models_match_migrations(self): - """Make sure that no model changes exist. + """Make sure that no pending migrations exist for the app. - This logic is taken from django's makemigrations.py file. Here just detect if model changes exist that require a migration, and if so we fail. """ - app_labels = ['django_celery_results'] - loader = MigrationLoader(None, ignore_no_migrations=True) - questioner = NonInteractiveMigrationQuestioner( - specified_apps=app_labels, dry_run=False) - autodetector = MigrationAutodetector( - loader.project_state(), - ProjectState.from_apps(apps), - questioner, + call_command( + "makemigrations", "django_celery_results", "--check", "--dry-run" ) - changes = autodetector.changes( - graph=loader.graph, - trim_to_apps=app_labels, - convert_apps=app_labels, - migration_name='fake_name', - ) - self.assertTrue( - not changes, - msg='Model changes exist that do not have a migration') + + @override_settings(DEFAULT_AUTO_FIELD='django.db.models.BigAutoField') + def test_models_match_migrations_with_changed_default_auto_field(self): + """Test with changing default_auto_field. + + This logic make sure that no pending migrations created even if + the user changes the `DEFAULT_AUTO_FIELD`. + """ + self.test_models_match_migrations() diff --git a/t/unit/test_utils.py b/t/unit/test_utils.py new file mode 100644 index 00000000..344f1449 --- /dev/null +++ b/t/unit/test_utils.py @@ -0,0 +1,41 @@ +from celery import uuid +from django.test import TransactionTestCase + +from django_celery_results.models import TaskResult +from django_celery_results.utils import raw_delete + +_TEST_RECORDS_COUNT = 100 + + +class RawDeleteTest(TransactionTestCase): + """Unit test for `utils.raw_delete` func.""" + + def setUp(self): + # setup test results to be used against raw_delete + TaskResult.objects.bulk_create( + [TaskResult(task_id=uuid()) for _ in range(_TEST_RECORDS_COUNT)] + ) + assert TaskResult.objects.count() == _TEST_RECORDS_COUNT + + def _get_sample_result_ids(self, count): + return TaskResult.objects.values_list("pk").order_by("?")[:count] + + def test_all_rows_delete(self): + raw_delete( + queryset=TaskResult.objects.all() + ) + assert TaskResult.objects.count() == 0 + + def test_correct_rows_deleted(self): + # sample random 10 rows + sample_size = 10 + sample_ids = self._get_sample_result_ids(count=sample_size) + # update task_name to "test" + TaskResult.objects.filter(pk__in=sample_ids).update(task_name="test") + # "raw delete" results with name = "test" + raw_delete( + queryset=TaskResult.objects.filter(task_name="test") + ) + assert TaskResult.objects.filter(task_name="test").count() == 0 + results_remaining = _TEST_RECORDS_COUNT - sample_size + assert TaskResult.objects.count() == results_remaining