diff --git a/HISTORY.rst b/HISTORY.rst index 11a4e9f..00ab888 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -1,16 +1,26 @@ -Pending -******* +5.0 (TBD) +********* * Add support for Django 5.1. * Drop support for Django 3.2, 4.0, 4.1. * Convert ``Safe`` to be a custom class rather than an ``Enum``. -* The valid values for ``safe`` are: +* The standard values of ``Safe`` now support being called. For example: * ``Safe.before_deploy()`` * ``Safe.after_deploy()`` * ``Safe.always()`` * Add support for allowing a ``Safe.after_deploy(delay=timedelta())`` migration to be migrated after the delay has passed. +* Change the default value of ``safe`` to ``Safe.always``. + This is a better default for third party apps that are not using + ``django_safemigrate``. +* ``Safe.after_deploy`` and ``Safe.always`` migrations will be + reported as blocked if they are behind a blocked ``Safe.before_deploy`` + migration. +* ``Safe.after_deploy`` migrations are now reported along with other + delayed migrations instead of being separately reported as protected. +* Use PEP8 compliant capitalization for enums internally. This doesn't + affect any documented API usage. 4.3 (2024-03-28) ++++++++++++++++ diff --git a/README.rst b/README.rst index accbbe6..c548e2d 100644 --- a/README.rst +++ b/README.rst @@ -45,7 +45,7 @@ such as a migration to add a column. from django_safemigrate import Safe class Migration(migrations.Migration): - safe = Safe.before_deploy() + safe = Safe.before_deploy At this point you can run the ``safemigrate`` Django command to run the migrations, and only these migrations will run. @@ -66,35 +66,32 @@ Safety Options There are three options for the value of the ``safe`` property of the migration. -* ``Safe.before_deploy()`` +* ``Safe.before_deploy`` This migration is only safe to run before the code change is deployed. For example, a migration that adds a new field to a model. -* ``Safe.after_deploy(delay=None)`` +* ``Safe.after_deploy`` This migration is only safe to run after the code change is deployed. - This is the default that is applied if no ``safe`` property is given. For example, a migration that removes a field from a model. By specifying a ``delay`` parameter, you can specify when a - ``Safe.after_deploy()`` migration can be run with the ``safemigrate`` + ``Safe.after_deploy`` migration can be run with the ``safemigrate`` command. For example, if it's desired to wait a week before applying a migration, you can specify ``Safe.after_deploy(delay=timedelta(days=7))``. - The ``delay`` is used with the datetime when the migration is first detected. - The detection datetime is when the ``safemigrate`` command detects the - migration in a plan that successfully runs. If the migration plan is blocked, - such when a ``Safe.after_deploy(delay=None)`` is in front of a - ``Safe.before_deploy()``, no migrations are marked as detected. - - Note that a ``Safe.after_deploy()`` migration will not run the first - time it's encountered. + The ``delay`` begins when safemigrate first delays the migration + without it being blocked, on a non-faking run that begins executing. + If the migration is blocked, such as if it is behind a ``Safe.before_deploy`` + migration that is behind another ``Safe.after_deploy`` migration, + no migrations are marked as detected. -* ``Safe.always()`` +* ``Safe.always`` This migration is safe to run before *and* after the code change is deployed. + This is the default that is applied if no ``safe`` property is given. For example, a migration that changes the ``help_text`` of a field. Pre-commit Hook @@ -122,8 +119,7 @@ Nonstrict Mode Under normal operation, if there are migrations that must run before the deployment that depend -on any migration that is marked to run after deployment -(or is not marked), +on any migration that is marked to run after deployment, the command will raise an error to indicate that there are protected migrations that should have already been run, but have not been, diff --git a/src/django_safemigrate/__init__.py b/src/django_safemigrate/__init__.py index cc1cf1c..42b29d7 100644 --- a/src/django_safemigrate/__init__.py +++ b/src/django_safemigrate/__init__.py @@ -5,25 +5,25 @@ from enum import Enum -class SafeEnum(Enum): - always = "always" - before_deploy = "before_deploy" - after_deploy = "after_deploy" +class When(Enum): + ALWAYS = "always" + BEFORE_DEPLOY = "before_deploy" + AFTER_DEPLOY = "after_deploy" @dataclass class Safe: - safe: SafeEnum + when: When delay: timedelta | None = None @classmethod def always(cls): - return cls(safe=SafeEnum.always) + return cls(when=When.ALWAYS) @classmethod def before_deploy(cls): - return cls(safe=SafeEnum.before_deploy) + return cls(when=When.BEFORE_DEPLOY) @classmethod def after_deploy(cls, *, delay: timedelta = None): - return cls(safe=SafeEnum.after_deploy, delay=delay) + return cls(when=When.AFTER_DEPLOY, delay=delay) diff --git a/src/django_safemigrate/management/commands/safemigrate.py b/src/django_safemigrate/management/commands/safemigrate.py index 054e1ce..29057a2 100644 --- a/src/django_safemigrate/management/commands/safemigrate.py +++ b/src/django_safemigrate/management/commands/safemigrate.py @@ -2,8 +2,10 @@ Migration safety is enforced by a pre_migrate signal receiver. """ + from __future__ import annotations +from functools import cached_property from enum import Enum from django.conf import settings @@ -14,67 +16,27 @@ from django.utils import timezone from django.utils.timesince import timeuntil -from django_safemigrate import Safe, SafeEnum +from django_safemigrate import Safe, When from django_safemigrate.models import SafeMigration -class SafeMigrate(Enum): - strict = "strict" - nonstrict = "nonstrict" - disabled = "disabled" - - -def safety(migration: Migration): - """Determine the safety status of a migration.""" - return getattr(migration, "safe", Safe.after_deploy()) - +class Mode(Enum): + """The mode of operation for safemigrate. -def safemigrate(): - state = getattr(settings, "SAFEMIGRATE", None) - if state is None: - return state - try: - return SafeMigrate(state.lower()) - except ValueError as e: - raise ValueError( - "Invalid SAFEMIGRATE setting, it must be one of 'strict', 'nonstrict', or 'disabled'." - ) from e + STRICT, the default mode, will throw an error if migrations + marked Safe.before_deploy() are blocked by unrun migrations that + are marked Safe.after_deploy() with an unfulfilled delay. + NONSTRICT will run the same migrations as strict mode, but will + not throw an error if migrations are blocked. -def filter_migrations( - migrations: list[Migration], -) -> tuple[list[Migration], list[Migration]]: + DISABLED will completely bypass safemigrate protections and run + exactly the same as the standard migrate command. """ - Filter migrations into ready and protected migrations. - - A protected migration is one that's marked Safe.after_deploy() - and has not yet passed its delay value. - """ - now = timezone.now() - - detected_map = SafeMigration.objects.get_detected_map( - [(m.app_label, m.name) for m in migrations] - ) - - def is_protected(migration): - migration_safe = safety(migration) - detected = detected_map.get((migration.app_label, migration.name)) - # A migration is protected if detected is None or delay is not specified. - return migration_safe.safe == SafeEnum.after_deploy and ( - detected is None - or migration_safe.delay is None - or now < (detected + migration_safe.delay) - ) - ready = [] - protected = [] - - for migration in migrations: - if is_protected(migration): - protected.append(migration) - else: - ready.append(migration) - return ready, protected + STRICT = "strict" + NONSTRICT = "nonstrict" + DISABLED = "disabled" class Command(migrate.Command): @@ -93,34 +55,89 @@ def handle(self, *args, **options): ) super().handle(*args, **options) - def pre_migrate_receiver(self, *, plan, **_): - """Handle the pre_migrate receiver for all apps.""" + def pre_migrate_receiver(self, *, plan: list[tuple[Migration, bool]], **_): + """Modify the migration plan to apply deployment safety.""" if self.receiver_has_run: - # Can't just look for the run for the current app, - # because it only sends the signal to apps with models. return # Only run once self.receiver_has_run = True - safemigrate_state = safemigrate() - if safemigrate_state == SafeMigrate.disabled: - # When disabled, run migrate - return - - # strict by default - strict = safemigrate_state != SafeMigrate.nonstrict + if self.mode == Mode.DISABLED: + return # Run migrate normally - if any(backward for mig, backward in plan): + if any(backward for _, backward in plan): raise CommandError("Backward migrations are not supported.") - # Pull the migrations into a new list - migrations = [migration for migration, backward in plan] + # Resolve the configuration for each migration + config = {migration: self.safe(migration) for migration, _ in plan} + + # Give a command error if any safe configuration is invalid + self.validate(config) + + # Get the dates of when migrations were detected + detected = self.detected(config) + + # Resolve the current status for each migration respecting delays + resolved = self.resolve(config, detected) + + # Categorize the migrations for display and action + ready, delayed, blocked = self.categorize(resolved) + + # Blocked migrations require delayed migrations + if not delayed: + return # Run all the migrations + + # Display the delayed migrations + self.write_delayed(delayed, config, resolved, detected) + + # Display the blocked migrations + if blocked: + self.write_blocked(blocked) + + if blocked and self.mode == Mode.STRICT: + raise CommandError("Aborting due to blocked migrations.") + + # Mark the delayed migrations as detected if not faking + if not self.fake: + self.detect( + [ + migration + for migration in delayed + if config[migration].when == When.AFTER_DEPLOY + and config[migration].delay is not None + ] + ) + + # Swap out the items in the plan with the safe migrations. + # None are backward, so we can always set backward to False. + plan[:] = [(migration, False) for migration in ready] + + @cached_property + def mode(self): + """Determine the configured mode of operation for safemigrate.""" + try: + return Mode(getattr(settings, "SAFEMIGRATE", "strict").lower()) + except ValueError: + raise ValueError( + "The SAFEMIGRATE setting is invalid." + " It must be one of 'strict', 'nonstrict', or 'disabled'." + ) + + @staticmethod + def safe(migration: Migration) -> Safe: + """Determine the safety setting of a migration.""" + callables = [Safe.before_deploy, Safe.after_deploy, Safe.always] + safe = getattr(migration, "safe", Safe.always) + return safe() if safe in callables else safe - # Check for invalid safe properties + def validate(self, config): + """Check for invalid safe configurations. + + Exit with a command error if any configurations are invalid. + """ invalid = [ migration - for migration in migrations - if not isinstance(safety(migration), Safe) - or safety(migration).safe not in SafeEnum + for migration, safe in config.items() + if not isinstance(safe, Safe) ] if invalid: self.stdout.write(self.style.MIGRATE_HEADING("Invalid migrations:")) @@ -130,88 +147,144 @@ def pre_migrate_receiver(self, *, plan, **_): "Aborting due to migrations with invalid safe properties." ) - ready, protected = filter_migrations(migrations) + def detected( + self, config: dict[Migration, Safe] + ) -> dict[Migration, timezone.datetime]: + """Get the detected dates for each migration.""" + detected_map = SafeMigration.objects.get_detected_map( + [ + (migration.app_label, migration.name) + for migration, safe in config.items() + if safe.when == When.AFTER_DEPLOY and safe.delay is not None + ] + ) + return { + migration: detected_map[(migration.app_label, migration.name)] + for migration in config + if (migration.app_label, migration.name) in detected_map + } + + def resolve( + self, + config: dict[Migration, Safe], + detected: dict[Migration, timezone.datetime], + ) -> dict[Migration, When]: + """Resolve the current status of each migration. + + ``When.AFTER_DEPLOY`` migrations are resolved to ``When.ALWAYS`` + if they have previously been detected and their delay has passed. + """ + now = timezone.now() + return { + migration: ( + When.ALWAYS + if safe.when == When.AFTER_DEPLOY + and safe.delay is not None + and migration in detected + and detected[migration] + safe.delay <= now + else safe.when + ) + for migration, safe in config.items() + } - if not protected: - return # No migrations to protect. + def categorize( + self, resolved: dict[Migration, When] + ) -> tuple[list[Migration], list[Migration], list[Migration]]: + """Categorize the migrations as ready, delayed, or blocked. - # Display the migrations that are protected - self.stdout.write(self.style.MIGRATE_HEADING("Protected migrations:")) - for migration in protected: - self.stdout.write(f" {migration.app_label}.{migration.name}") + Ready migrations are ready to be run immediately. + + Delayed migrations cannot be run immediately, but are safe to run + after deployment. - delayed = [] + Blocked migrations are dependent on a delayed migration, but + either need to run before deployment or depend on a migration + that needs to run before deployment. + """ + ready = [mig for mig, when in resolved.items() if when != When.AFTER_DEPLOY] + delayed = [mig for mig, when in resolved.items() if when == When.AFTER_DEPLOY] blocked = [] - while True: - blockers = protected + delayed + blocked - blockers_deps = [(m.app_label, m.name) for m in blockers] - to_block_deps = [dep for mig in blockers for dep in mig.run_before] - block = [ + if not delayed and not blocked: + return ready, delayed, blocked + + def to_block(target, blockers): + """Find a round of migrations that depend on these blockers.""" + blocker_deps = [(m.app_label, m.name) for m in blockers] + to_block_deps = [dep for m in blockers for dep in m.run_before] + return [ migration - for migration in ready - if any(dep in blockers_deps for dep in migration.dependencies) + for migration in target + if any(dep in blocker_deps for dep in migration.dependencies) or (migration.app_label, migration.name) in to_block_deps ] + + # Delay or block ready migrations that are behind delayed migrations. + while True: + block = to_block(ready, delayed + blocked) if not block: break for migration in block: ready.remove(migration) - if safety(migration).safe == SafeEnum.before_deploy: + if resolved[migration] == When.BEFORE_DEPLOY: blocked.append(migration) else: delayed.append(migration) - # Order the migrations in the order of the original plan. - delayed = [m for m in migrations if m in delayed] - blocked = [m for m in migrations if m in blocked] - - self.delayed(delayed) - self.blocked(blocked) + # Block delayed migrations that are behind other blocked migrations. + while True: + block = to_block(delayed, blocked) + if not block: + break - if blocked and strict: - raise CommandError("Aborting due to blocked migrations.") + for migration in block: + delayed.remove(migration) + blocked.append(migration) - # Only mark migrations as detected if not faking - if not self.fake: - # The detection datetime is what's used to determine if an - # after_deploy() with a delay can be migrated or not. - for migration in migrations: - SafeMigration.objects.get_or_create( - app=migration.app_label, name=migration.name + # Order the migrations in the order of the original plan. + ready = [m for m in resolved if m in ready] + delayed = [m for m in resolved if m in delayed] + blocked = [m for m in resolved if m in blocked] + + return ready, delayed, blocked + + def write_delayed( + self, + migrations: list[Migration], + config: dict[Migration, Safe], + resolved: dict[Migration, When], + detected: dict[Migration, timezone.datetime], + ): + """Display delayed migrations.""" + self.stdout.write(self.style.MIGRATE_HEADING("Delayed migrations:")) + for migration in migrations: + if ( + resolved[migration] == When.AFTER_DEPLOY + and config[migration].delay is not None + ): + now = timezone.localtime() + migrate_date = detected.get(migration, now) + config[migration].delay + humanized_date = timeuntil(migrate_date, now=now, depth=2) + self.stdout.write( + f" {migration.app_label}.{migration.name} " + f"(can automatically migrate in {humanized_date} " + f"- {migrate_date.isoformat()})" ) + else: + self.stdout.write(f" {migration.app_label}.{migration.name}") - # Swap out the items in the plan with the safe migrations. - # None are backward, so we can always set backward to False. - plan[:] = [(migration, False) for migration in ready] + def write_blocked(self, migrations: list[Migration]): + """Display blocked migrations.""" + self.stdout.write(self.style.MIGRATE_HEADING("Blocked migrations:")) + for migration in migrations: + self.stdout.write(f" {migration.app_label}.{migration.name}") - def delayed(self, migrations): - """Handle delayed migrations.""" - # Display delayed migrations if they exist: - if migrations: - self.stdout.write(self.style.MIGRATE_HEADING("Delayed migrations:")) - for migration in migrations: - migration_safe = safety(migration) - if ( - migration_safe.safe == SafeEnum.after_deploy - and migration_safe.delay is not None - ): - now = timezone.localtime() - migrate_date = now + migration_safe.delay - humanized_date = timeuntil(migrate_date, now=now, depth=2) - self.stdout.write( - f" {migration.app_label}.{migration.name} " - f"(can automatically migrate in {humanized_date} " - f"- {migrate_date.isoformat()})" - ) - else: - self.stdout.write(f" {migration.app_label}.{migration.name}") - - def blocked(self, migrations): - """Handle blocked migrations.""" - # Display blocked migrations if they exist. - if migrations: - self.stdout.write(self.style.MIGRATE_HEADING("Blocked migrations:")) - for migration in migrations: - self.stdout.write(f" {migration.app_label}.{migration.name}") + def detect(self, migrations): + """Mark the given migrations as detected.""" + # The detection datetime is what's used to determine if an + # after_deploy() with a delay can be migrated or not. + for migration in migrations: + SafeMigration.objects.get_or_create( + app=migration.app_label, name=migration.name + ) diff --git a/src/django_safemigrate/models.py b/src/django_safemigrate/models.py index a5eaff4..02cfdb3 100644 --- a/src/django_safemigrate/models.py +++ b/src/django_safemigrate/models.py @@ -7,7 +7,9 @@ class SafeMigrationManager(models.Manager): - def get_detected_map(self, app_model_pairs: list[tuple[str, str]]): + def get_detected_map( + self, app_model_pairs: list[tuple[str, str]] + ) -> dict[tuple[str, str], timezone.datetime]: detection_qs = ( self.get_queryset() .annotate( @@ -34,7 +36,9 @@ class Meta: name = models.CharField(max_length=255) detected = models.DateTimeField( help_text=_( - "The time the migration was detected. This is used to determine when a migration with Safe.after_deploy() should be migrated." + "The time the migration was detected." + " This is used to determine when a migration with" + " Safe.after_deploy() should be migrated." ), default=timezone.now, ) diff --git a/tests/safemigrate_test.py b/tests/safemigrate_test.py index 357b25d..8cd7387 100644 --- a/tests/safemigrate_test.py +++ b/tests/safemigrate_test.py @@ -1,4 +1,5 @@ """Unit tests for the safemigrate command.""" + from datetime import timedelta from io import StringIO @@ -55,6 +56,8 @@ def test_receiver_addition(self, mocker): def test_rerun(self, receiver): """Avoid running the pre_migrate_receiver twice.""" + # Make sure we're looking for the right attribute + assert receiver.__self__.receiver_has_run == False receiver.__self__.receiver_has_run = True plan = [(Migration(), False)] receiver(plan=plan) @@ -78,6 +81,16 @@ def test_all_before(self, receiver): receiver(plan=plan) assert len(plan) == 1 + def test_accept_uncalled(self, receiver): + """Uncalled Safe classmethods are accepted for backward compatibility.""" + plan = [ + (Migration("a", "0001_initial", safe=Safe.before_deploy), False), + (Migration("b", "0001_initial", safe=Safe.always), False), + (Migration("c", "0001_initial", safe=Safe.after_deploy), False), + ] + receiver(plan=plan) + assert len(plan) == 2 + def test_final_after(self, receiver): """Run everything except the final migration.""" plan = [ @@ -257,26 +270,33 @@ def test_after_doesnt_apply_on_first_run(self, receiver): receiver(plan=plan) assert len(plan) == 0 - def test_after_message(self, receiver): + def test_after_message(self): """ Confirm the delayed messaging of a migration with an after_deploy safety. """ - migrations = [ - Migration( - "spam", - "0001_initial", - safe=Safe.after_deploy(delay=(timedelta(days=8))), - ), - Migration( - "spam", - "0002_followup", - safe=Safe.after_deploy(), - dependencies=[("spam", "0001_initial")], + plan = [ + ( + Migration( + "spam", + "0001_initial", + safe=Safe.after_deploy(delay=(timedelta(days=8))), + ), + False, + ), + ( + Migration( + "spam", + "0002_followup", + safe=Safe.after_deploy(), + dependencies=[("spam", "0001_initial")], + ), + False, ), ] out = StringIO() - Command(stdout=out).delayed(migrations) + receiver = Command(stdout=out).pre_migrate_receiver + receiver(plan=plan) result = out.getvalue().strip() header, migration1, migration2 = result.split("\n", maxsplit=2) assert header == "Delayed migrations:" @@ -436,16 +456,14 @@ def test_blocked_by_after_nonstrict(self, settings, receiver): assert len(plan) == 1 def test_with_non_safe_migration_nonstrict(self, settings, receiver): - """ - Nonstrict mode allows prevents protected migrations. - In this case, that's migrations without a safe attribute. - """ + """Run ready deployments even if there are blocked migrations.""" settings.SAFEMIGRATE = "nonstrict" plan = [ ( Migration( "auth", "0001_initial", + safe=Safe.after_deploy(), ), False, ), @@ -526,12 +544,12 @@ def test_boolean_invalid(self, receiver): def test_migrations_not_detected_when_blocked(self, receiver): """If the plan can't advance, the migrations shouldn't be marked as detected.""" plan = [ - (Migration("spam", "0001_initial", safe=Safe.before_deploy()), False), + (Migration("spam", "0001_initial", safe=Safe.before_deploy), False), ( Migration( "spam", "0002_followup", - safe=Safe.after_deploy(), + safe=Safe.after_deploy(delay=timedelta(days=1)), dependencies=[("spam", "0001_initial")], ), False, @@ -540,16 +558,67 @@ def test_migrations_not_detected_when_blocked(self, receiver): Migration( "spam", "0003_safety", - safe=Safe.before_deploy(), + safe=Safe.before_deploy, dependencies=[("spam", "0002_followup")], ), False, ), + ( + Migration( + "spam", + "0004_blocked", + safe=Safe.after_deploy(delay=timedelta(days=1)), + dependencies=[("spam", "0003_safety")], + ), + False, + ), ] with pytest.raises(CommandError): receiver(plan=plan) assert not SafeMigration.objects.exists() + def test_migrations_detected_when_blocked_nonstrict(self, settings, receiver): + """Delayed migrations should be detected even if others are blocked. + + This behavior is unique to nonstrict mode, where the migrations + still run even if there are blocked migrations. + """ + settings.SAFEMIGRATE = "nonstrict" + + plan = [ + (Migration("spam", "0001_initial", safe=Safe.before_deploy), False), + ( + Migration( + "spam", + "0002_followup", + safe=Safe.after_deploy(delay=timedelta(days=1)), + dependencies=[("spam", "0001_initial")], + ), + False, + ), + ( + Migration( + "spam", + "0003_safety", + safe=Safe.before_deploy, + dependencies=[("spam", "0002_followup")], + ), + False, + ), + ( + Migration( + "spam", + "0004_blocked", + safe=Safe.after_deploy(delay=timedelta(days=1)), + dependencies=[("spam", "0003_safety")], + ), + False, + ), + ] + receiver(plan=plan) + assert len(plan) == 1 + assert SafeMigration.objects.count() == 1 + def test_migrations_not_detected_when_faked(self, receiver): """If migrate command is faked, the migrations shouldn't be marked as detected.""" plan = [ @@ -558,7 +627,7 @@ def test_migrations_not_detected_when_faked(self, receiver): Migration( "spam", "0002_followup", - safe=Safe.after_deploy(), + safe=Safe.after_deploy(delay=timedelta(days=1)), dependencies=[("spam", "0001_initial")], ), False, @@ -567,13 +636,15 @@ def test_migrations_not_detected_when_faked(self, receiver): Migration( "spam", "0003_safety", - safe=Safe.after_deploy(), + safe=Safe.after_deploy(delay=timedelta(days=2)), dependencies=[("spam", "0002_followup")], ), False, ), ] command = Command() + # Make sure we're looking for the right attribute + assert command.fake == False command.fake = True command.pre_migrate_receiver(plan=plan) assert SafeMigration.objects.count() == 0 @@ -588,8 +659,8 @@ def test_migrations_are_detected(self, receiver): ( Migration( "spam", - "0002_followup", - safe=Safe.after_deploy(), + "0002_delayed", + safe=Safe.after_deploy(delay=timedelta(days=1)), dependencies=[("spam", "0001_initial")], ), False, @@ -605,9 +676,9 @@ def test_migrations_are_detected(self, receiver): ), ] receiver(plan=plan) - assert SafeMigration.objects.count() == 3 + assert SafeMigration.objects.count() == 2 # Confirm the existing value is not updated - assert SafeMigration.objects.filter(detected__gt=existing.detected).count() == 2 + assert SafeMigration.objects.filter(detected__gt=existing.detected).count() == 1 class TestCheckMissingSafe: