Skip to content

Commit

Permalink
Add pre-commit hook
Browse files Browse the repository at this point in the history
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
#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 <schillingt@better-simple.com>
  • Loading branch information
ryanhiebert and tim-schilling committed Sep 13, 2023
1 parent e4e5300 commit 4e76e8c
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 0 deletions.
9 changes: 9 additions & 0 deletions .pre-commit-hooks.yaml
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
56 changes: 56 additions & 0 deletions src/django_safemigrate/check.py
Original file line number Diff line number Diff line change
@@ -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<MigrationClass>\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()
35 changes: 35 additions & 0 deletions tests/safemigrate_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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"])

0 comments on commit 4e76e8c

Please sign in to comment.