Skip to content

Commit

Permalink
feat: give users the option to run the json migration asyncly
Browse files Browse the repository at this point in the history
  • Loading branch information
aqeelat committed Jan 23, 2023
1 parent 2fe9614 commit 890a3d8
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 47 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

#### Breaking Changes

- feat: Change `LogEntry.change` field type to `JSONField` rather than `TextField`. This change include a migration that may take time to run depending on the number of records on your `LogEntry` table ([#407](https://github.com/jazzband/django-auditlog/pull/407))
- feat: Change `LogEntry.change` field type to `JSONField` rather than `TextField`. This change include a migration that may take time to run depending on the number of records on your `LogEntry` table ([#407](https://github.com/jazzband/django-auditlog/pull/407))([#495](https://github.com/jazzband/django-auditlog/pull/495))

#### Improvements

Expand Down
16 changes: 10 additions & 6 deletions auditlog/management/commands/auditlogmigratejson.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from math import ceil

from django.conf import settings
from django.core.management.base import BaseCommand
from django.core.paginator import Paginator

from auditlog.models import LogEntry

Expand All @@ -13,16 +14,18 @@ def add_arguments(self, parser):
"-d",
"--database",
default=None,
help="If provided, we will use native db operations.",
help="If provided, the script will use native db operations. "
"Otherwise, it will use LogEntry.objects.bulk_create",
dest="db",
type=str,
choices=["postgres", "mysql", "oracle"],
)
parser.add_argument(
"-b",
"--bactch-size",
default=None,
help="Split the migration into multiple batches",
default=500,
help="Split the migration into multiple batches. If 0, then no batching will be done. "
"When passing a -d/database, the batch value will be ignored.",
dest="batch_size",
type=int,
)
Expand Down Expand Up @@ -83,12 +86,13 @@ def _apply_django_migration(_logs) -> int:
return len(updated)

logs = self.get_logs()

if not batch_size:
return _apply_django_migration(logs)

total_updated = 0
for page in Paginator(logs, batch_size):
total_updated += _apply_django_migration(page.object_list)
for _ in range(ceil(logs.count() / batch_size)):
total_updated += _apply_django_migration(self.get_logs()[:batch_size])
return total_updated

def migrate_using_sql(self, database):
Expand Down
4 changes: 1 addition & 3 deletions auditlog/migrations/0015_alter_logentry_changes.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def two_step_migrations() -> List:
migrations.AddField(
model_name="logentry",
name="changes_text",
field=models.TextField(null=True, verbose_name="change message"),
field=models.TextField(blank=True, verbose_name="text change message"),
),
migrations.AlterField(
model_name="logentry",
Expand All @@ -40,6 +40,4 @@ class Migration(migrations.Migration):
("auditlog", "0014_logentry_cid"),
]

atomic = not settings.AUDITLOG_TWO_STEP_MIGRATION

operations = [*two_step_migrations()]
2 changes: 1 addition & 1 deletion auditlog/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ class Action:
action = models.PositiveSmallIntegerField(
choices=Action.choices, verbose_name=_("action"), db_index=True
)
changes_text = models.TextField(null=True, verbose_name=_("change message"))
changes_text = models.TextField(blank=True, verbose_name=_("change message"))
changes = models.JSONField(null=True, verbose_name=_("change message"))
actor = models.ForeignKey(
to=settings.AUTH_USER_MODEL,
Expand Down
39 changes: 3 additions & 36 deletions auditlog_tests/test_two_step_json_migration.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import json
import unittest
from io import StringIO
from unittest.mock import patch

Expand Down Expand Up @@ -35,8 +34,8 @@ def test_use_text_changes_first(self):


class AuditlogMigrateJsonTest(TestCase):
def make_logentry(self, text="I am a simple model."):
model = SimpleModel.objects.create(text=text)
def make_logentry(self):
model = SimpleModel.objects.create(text="I am a simple model.")
log_entry: LogEntry = model.history.first()
log_entry.changes_text = json.dumps(log_entry.changes)
log_entry.changes = None
Expand Down Expand Up @@ -73,7 +72,7 @@ def test_using_django(self):
log_entry = self.make_logentry()

# Act
outbuf, errbuf = self.call_command()
outbuf, errbuf = self.call_command("-b=0")
log_entry.refresh_from_db()

# Assert
Expand All @@ -85,42 +84,10 @@ def test_using_django_batched(self):
log_entry_1 = self.make_logentry()
log_entry_2 = self.make_logentry()

# Act
outbuf, errbuf = self.call_command("-b=2")
log_entry_1.refresh_from_db()
log_entry_2.refresh_from_db()

# Assert
self.assertEqual(errbuf, "")
self.assertIsNotNone(log_entry_1.changes)
self.assertIsNotNone(log_entry_2.changes)

@unittest.skip("Flaky test descriped below.")
def test_using_django_batched_with_batch_size_1(self):
"""
Weird things happen when I use b=1.
When I click debug, the test works, but when I click run, it fails.
What happens when I click run is:
1. printing the logs variable returns all 3 logs in the correct order
2. the last page's `page.object_list` is an empty queryset.
3. page 1 has l1 and page 2 has l3 (no l2)
Changing the batch size to 2, page 1 has l1 and l2, and page 2 is empty.
Could this be an issue with django's paginator? I don't know.
Anyways, I don't think this is an issue as users can run the script twice and it should work.
"""

# Arrange
log_entry_1 = self.make_logentry("l1")
log_entry_2 = self.make_logentry("l2")
log_entry_3 = self.make_logentry("l3")

# Act
outbuf, errbuf = self.call_command("-b=1")
log_entry_1.refresh_from_db()
log_entry_2.refresh_from_db()
log_entry_3.refresh_from_db()

# Assert
self.assertEqual(errbuf, "")
Expand Down

0 comments on commit 890a3d8

Please sign in to comment.