From 4e76e8ccaf6d4cb944f1e8b357d695b9634bc20c Mon Sep 17 00:00:00 2001 From: Ryan Hiebert Date: Mon, 11 Sep 2023 14:50:02 -0500 Subject: [PATCH] Add pre-commit hook Check the migrations files in the current repository and ensure that they all have a `safe` property defined. While we can think of easy ways to break this implementation, for all the use-cases I've had this covers it quite nicely so I think it's worth merging even if its rough. Tim Schilling put an initial sketch of this in https://github.com/aspiredu/django-safemigrate/issues/39 and I was able to modify it for use in a precommit hook without difficulty. The modifications were made to tailor it to the environment of pre-commit, where file names are passed to the command as arguments. This allows pre-commit to only need to run the checks incrementally for the affected files for new commits. Co-authored-by: Tim Schilling --- .pre-commit-hooks.yaml | 9 ++++++ pyproject.toml | 3 ++ src/django_safemigrate/check.py | 56 +++++++++++++++++++++++++++++++++ tests/safemigrate_test.py | 35 +++++++++++++++++++++ 4 files changed, 103 insertions(+) create mode 100644 .pre-commit-hooks.yaml create mode 100644 src/django_safemigrate/check.py diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml new file mode 100644 index 0000000..0107a45 --- /dev/null +++ b/.pre-commit-hooks.yaml @@ -0,0 +1,9 @@ +- id: check + name: Check migrations safety + description: Ensure that all local migrations have been marked for safety. + entry: safemigrate-check + language: python + files: migrations/\d{4}_.+\.py$ + types: [file, python, text] + stages: [pre-commit, pre-merge-commit, pre-push, manual] + pass_filenames: true diff --git a/pyproject.toml b/pyproject.toml index a52de75..8be4678 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -33,3 +33,6 @@ django = ">=3.2,<5.0" [tool.poetry.dev-dependencies] tox = "*" + +[tool.poetry.plugins."console_scripts"] +"safemigrate-check" = "django_safemigrate.check:main" diff --git a/src/django_safemigrate/check.py b/src/django_safemigrate/check.py new file mode 100644 index 0000000..c4c3960 --- /dev/null +++ b/src/django_safemigrate/check.py @@ -0,0 +1,56 @@ +""" +Ensure migrations are using django_safemigrate. + +This is fairly rudimentary and won't work if the class doesn't +explicitly inherit from ``Migration``. +""" +import re +import sys + +MIGRATION_PATTERN = re.compile(r"class\s+(?P\w+)\s?\(.*Migration\):") + +MISSING_SAFE_MESSAGE = ( + "{file_path}: {migration_class} is missing the 'safe' attribute.\n" +) +FAILURE_MESSAGE = ( + "\n" + "Add the following to the migration class:\n" + "\n" + "from django_safemigrate import Safe\n" + "class Migration(migrations.Migration):\n" + " safe = Safe.before_deploy\n" + "\n" + "You can also use the following:\n" + " safe = Safe.always\n" + " safe = Safe.after_deploy\n" + "\n" +) + + +def validate_migrations(files): + success = True + for file_path in files: + with open(file_path) as f: + content = f.read() + + match = MIGRATION_PATTERN.search(content) + if match: + migration_class = match.group("MigrationClass") + if "safe = Safe." not in content: + success = False + sys.stdout.write( + MISSING_SAFE_MESSAGE.format( + file_path=file_path, migration_class=migration_class + ) + ) + if not success: + sys.stdout.write(FAILURE_MESSAGE) + return success + + +def main(): # pragma: no cover + sys.exit(0 if validate_migrations(sys.argv[1:]) else 1) + + +if __name__ == "__main__": # pragma: no cover + main() diff --git a/tests/safemigrate_test.py b/tests/safemigrate_test.py index 36702ff..be9aa50 100644 --- a/tests/safemigrate_test.py +++ b/tests/safemigrate_test.py @@ -3,6 +3,7 @@ from django.core.management.base import CommandError from django_safemigrate import Safe +from django_safemigrate.check import validate_migrations from django_safemigrate.management.commands.safemigrate import Command @@ -265,3 +266,37 @@ def test_boolean_invalid(self, receiver): plan = [(Migration("spam", "0001_initial", safe=False), False)] with pytest.raises(CommandError): receiver(plan=plan) + + +class TestCheck: + """Exercise the check command.""" + + MARKED = """ +from django.db import migrations +from django_safemigrate import Safe + +class Migration(migrations.Migration): + safe = Safe.always +""" + + UNMARKED = """ +from django.db import migrations + +class Migration(migrations.Migration): + pass +""" + + def test_validate_migrations_success(self, tmp_path): + with open(tmp_path / "0001_initial.py", "w") as f: + f.write(self.MARKED) + assert validate_migrations([tmp_path / "0001_initial.py"]) + + def test_validate_migrations_failure(self, tmp_path): + with open(tmp_path / "0001_initial.py", "w") as f: + f.write(self.UNMARKED) + assert not validate_migrations([tmp_path / "0001_initial.py"]) + + def test_validate_migrations_falsematch(self, tmp_path): + with open(tmp_path / "0001_initial.py", "w") as f: + f.write("THIS IS NOT A MIGRATION") + assert validate_migrations([tmp_path / "0001_initial.py"])