Skip to content

Commit

Permalink
feat: raw delete expired instead of Queryset.delete (#235)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
daadu committed Oct 11, 2021
1 parent 1a52d15 commit 7c7cb67
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 27 deletions.
4 changes: 2 additions & 2 deletions django_celery_results/managers.py
Expand Up @@ -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'
Expand Down Expand Up @@ -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):
Expand Down
5 changes: 5 additions & 0 deletions django_celery_results/utils.py
Expand Up @@ -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)
39 changes: 14 additions & 25 deletions 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

Expand All @@ -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()
41 changes: 41 additions & 0 deletions 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

0 comments on commit 7c7cb67

Please sign in to comment.