-
Notifications
You must be signed in to change notification settings - Fork 12
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(cleanup): convert cleanup migration to command (#467)
The cleanup migration is too dangerous since it will possibly delete production data if something goes wrong. This commit converts the migration to a command which can be called manually and has a dry run option to test it beforehand.
- Loading branch information
Showing
5 changed files
with
75 additions
and
24 deletions.
There are no files selected for viewing
Empty file.
Empty file.
39 changes: 39 additions & 0 deletions
39
document_merge_service/api/management/commands/clean_dangling_files.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
import os | ||
|
||
from django.conf import settings | ||
from django.core.management.base import BaseCommand | ||
|
||
from document_merge_service.api.models import Template | ||
|
||
|
||
class Command(BaseCommand): | ||
help = "Remove dangling template files that are not attached to a template model anymore" | ||
|
||
def add_arguments(self, parser): | ||
parser.add_argument("--dry-run", dest="dry", action="store_true", default=False) | ||
|
||
def handle(self, *args, **options): | ||
used_files = [template.template.path for template in Template.objects.all()] | ||
|
||
for subdir, dirs, files in os.walk(settings.MEDIA_ROOT): | ||
for f in files: | ||
path = os.path.join(subdir, f) | ||
if path not in used_files and os.path.isfile(path): | ||
try: | ||
if not options.get("dry"): | ||
os.remove(path) | ||
self.stdout.write( | ||
self.style.SUCCESS(f"Deleted dangling file '{path}'") | ||
) | ||
else: | ||
self.stdout.write( | ||
self.style.WARNING( | ||
f"Would delete dangling file '{path}'" | ||
) | ||
) | ||
except Exception as e: # pragma: no cover | ||
self.stdout.write( | ||
self.style.ERROR( | ||
f"Could not delete dangling file '{path}': {str(e)}" | ||
) | ||
) |
26 changes: 2 additions & 24 deletions
26
document_merge_service/api/migrations/0004_cleanup_files.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,28 +1,6 @@ | ||
import os | ||
import logging | ||
|
||
|
||
from django.db import migrations | ||
from django.conf import settings | ||
|
||
|
||
def cleanup_files(apps, schema_editor): | ||
Template = apps.get_model("api", "Template") | ||
used_files = [template.template.path for template in Template.objects.all()] | ||
|
||
with os.scandir(settings.MEDIA_ROOT) as files: | ||
for f in files: | ||
if not f.path in used_files and os.path.isfile(f.path): | ||
try: | ||
os.remove(f.path) | ||
except Exception as e: | ||
pass # ignore permission issues when running this in a fresh container | ||
|
||
|
||
class Migration(migrations.Migration): | ||
|
||
dependencies = [ | ||
("api", "0003_template_meta"), | ||
] | ||
|
||
operations = [migrations.RunPython(cleanup_files, migrations.RunPython.noop)] | ||
dependencies = [("api", "0003_template_meta")] | ||
operations = [] |
34 changes: 34 additions & 0 deletions
34
document_merge_service/api/tests/test_clean_dangling_files.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
import os | ||
|
||
import pytest | ||
from django.core.management import call_command | ||
|
||
from document_merge_service.api.data import django_file | ||
|
||
|
||
@pytest.mark.parametrize("dry", [True, False]) | ||
def test_clean_dangling_files(db, dry, settings, template_factory): | ||
templates = [ | ||
template_factory(template=django_file("docx-template.docx")), | ||
template_factory(template=django_file("docx-template-syntax.docx")), | ||
] | ||
dangling_files = [ | ||
django_file("docx-template-filters.docx"), | ||
django_file("docx-template-loopcontrols.docx"), | ||
] | ||
|
||
call_command("clean_dangling_files", dry=dry) | ||
|
||
assert ( | ||
all( | ||
[ | ||
os.path.isfile(os.path.join(settings.MEDIA_ROOT, file.name)) is dry | ||
for file in dangling_files | ||
] | ||
) | ||
is True | ||
) | ||
assert ( | ||
all([os.path.isfile(template.template.path) is True for template in templates]) | ||
is True | ||
) |