From 50931dfa5310d5ae1c6e2852d05bf1e86700827f Mon Sep 17 00:00:00 2001 From: Mounir Messelmeni Date: Fri, 12 Feb 2016 11:02:36 +0100 Subject: [PATCH] Fixed #25304 -- Allowed management commands to check if migrations are applied. --- .../management/commands/changepassword.py | 2 +- .../management/commands/createsuperuser.py | 1 + django/core/management/base.py | 43 ++++++++++++++++++- django/core/management/commands/runserver.py | 37 +--------------- docs/howto/custom-management-commands.txt | 8 ++++ docs/releases/1.10.txt | 6 +++ tests/admin_scripts/tests.py | 2 +- tests/user_commands/tests.py | 16 ++++++- 8 files changed, 76 insertions(+), 39 deletions(-) diff --git a/django/contrib/auth/management/commands/changepassword.py b/django/contrib/auth/management/commands/changepassword.py index 162d08a3a91..fd1deada0f9 100644 --- a/django/contrib/auth/management/commands/changepassword.py +++ b/django/contrib/auth/management/commands/changepassword.py @@ -12,7 +12,7 @@ from django.utils.encoding import force_str class Command(BaseCommand): help = "Change a user's password for django.contrib.auth." - + requires_migrations_checks = True requires_system_checks = False def _get_pass(self, prompt="Password: "): diff --git a/django/contrib/auth/management/commands/createsuperuser.py b/django/contrib/auth/management/commands/createsuperuser.py index 58364ec543e..d318c114e6f 100644 --- a/django/contrib/auth/management/commands/createsuperuser.py +++ b/django/contrib/auth/management/commands/createsuperuser.py @@ -23,6 +23,7 @@ class NotRunningInTTYException(Exception): class Command(BaseCommand): help = 'Used to create a superuser.' + requires_migrations_checks = True def __init__(self, *args, **kwargs): super(Command, self).__init__(*args, **kwargs) diff --git a/django/core/management/base.py b/django/core/management/base.py index 0d71e0f3d4c..ce9ba0be48c 100644 --- a/django/core/management/base.py +++ b/django/core/management/base.py @@ -11,8 +11,10 @@ from argparse import ArgumentParser import django from django.core import checks +from django.core.exceptions import ImproperlyConfigured from django.core.management.color import color_style, no_style -from django.db import connections +from django.db import DEFAULT_DB_ALIAS, connections +from django.db.migrations.exceptions import MigrationSchemaMissing from django.utils.encoding import force_str @@ -165,6 +167,10 @@ class BaseCommand(object): wrapped with ``BEGIN;`` and ``COMMIT;``. Default value is ``False``. + ``requires_migrations_checks`` + A boolean; if ``True``, the command prints a warning if the set of + migrations on disk don't match the migrations in the database. + ``requires_system_checks`` A boolean; if ``True``, entire Django project will be checked for errors prior to executing the command. Default value is ``True``. @@ -199,6 +205,7 @@ class BaseCommand(object): can_import_settings = True output_transaction = False # Whether to wrap the output in a "BEGIN; COMMIT;" leave_locale_alone = False + requires_migrations_checks = False requires_system_checks = True def __init__(self, stdout=None, stderr=None, no_color=False): @@ -336,6 +343,8 @@ class BaseCommand(object): try: if self.requires_system_checks and not options.get('skip_checks'): self.check() + if self.requires_migrations_checks: + self.check_migrations() output = self.handle(*args, **options) if output: if self.output_transaction: @@ -419,6 +428,38 @@ class BaseCommand(object): else: self.stdout.write(msg) + def check_migrations(self): + """ + Print a warning if the set of migrations on disk don't match the + migrations in the database. + """ + from django.db.migrations.executor import MigrationExecutor + try: + executor = MigrationExecutor(connections[DEFAULT_DB_ALIAS]) + except ImproperlyConfigured: + # No databases are configured (or the dummy one) + return + except MigrationSchemaMissing: + self.stdout.write(self.style.NOTICE( + "\nNot checking migrations as it is not possible to access/create the django_migrations table." + )) + return + + plan = executor.migration_plan(executor.loader.graph.leaf_nodes()) + if plan: + apps_waiting_migration = sorted(set(migration.app_label for migration, backwards in plan)) + self.stdout.write( + self.style.NOTICE( + "\nYou have %(unpplied_migration_count)s unapplied migration(s). " + "Your project may not work properly until you apply the " + "migrations for app(s): %(apps_waiting_migration)s." % { + "unpplied_migration_count": len(plan), + "apps_waiting_migration": ", ".join(apps_waiting_migration), + } + ) + ) + self.stdout.write(self.style.NOTICE("Run 'python manage.py migrate' to apply them.\n")) + def handle(self, *args, **options): """ The actual logic of the command. Subclasses must implement diff --git a/django/core/management/commands/runserver.py b/django/core/management/commands/runserver.py index 68de4ac6e54..1f5b52f1138 100644 --- a/django/core/management/commands/runserver.py +++ b/django/core/management/commands/runserver.py @@ -8,12 +8,8 @@ import sys from datetime import datetime from django.conf import settings -from django.core.exceptions import ImproperlyConfigured from django.core.management.base import BaseCommand, CommandError from django.core.servers.basehttp import get_internal_wsgi_application, run -from django.db import DEFAULT_DB_ALIAS, connections -from django.db.migrations.exceptions import MigrationSchemaMissing -from django.db.migrations.executor import MigrationExecutor from django.utils import autoreload, six from django.utils.encoding import force_text, get_system_encoding @@ -114,6 +110,8 @@ class Command(BaseCommand): self.stdout.write("Performing system checks...\n\n") self.check(display_num_errors=True) + # Need to check migrations here, so can't use the + # requires_migrations_check attribute. self.check_migrations() now = datetime.now().strftime('%B %d, %Y - %X') if six.PY2: @@ -154,36 +152,5 @@ class Command(BaseCommand): self.stdout.write(shutdown_message) sys.exit(0) - def check_migrations(self): - """ - Checks to see if the set of migrations on disk matches the - migrations in the database. Prints a warning if they don't match. - """ - try: - executor = MigrationExecutor(connections[DEFAULT_DB_ALIAS]) - except ImproperlyConfigured: - # No databases are configured (or the dummy one) - return - except MigrationSchemaMissing: - self.stdout.write(self.style.NOTICE( - "\nNot checking migrations as it is not possible to access/create the django_migrations table." - )) - return - - plan = executor.migration_plan(executor.loader.graph.leaf_nodes()) - if plan: - apps_waiting_migration = sorted(set(migration.app_label for migration, backwards in plan)) - self.stdout.write( - self.style.NOTICE( - "\nYou have %(unpplied_migration_count)s unapplied migration(s). " - "Your project may not work properly until you apply the " - "migrations for app(s): %(apps_waiting_migration)s." % { - "unpplied_migration_count": len(plan), - "apps_waiting_migration": ", ".join(apps_waiting_migration), - } - ) - ) - self.stdout.write(self.style.NOTICE("Run 'python manage.py migrate' to apply them.\n")) - # Kept for backward compatibility BaseRunserverCommand = Command diff --git a/docs/howto/custom-management-commands.txt b/docs/howto/custom-management-commands.txt index 36baa629ed9..cf84b498ef4 100644 --- a/docs/howto/custom-management-commands.txt +++ b/docs/howto/custom-management-commands.txt @@ -231,6 +231,14 @@ All attributes can be set in your derived class and can be used in ``True``, the output will automatically be wrapped with ``BEGIN;`` and ``COMMIT;``. Default value is ``False``. +.. attribute:: BaseCommand.requires_migrations_checks + + .. versionadded:: 1.10 + + A boolean; if ``True``, the command prints a warning if the set of + migrations on disk don't match the migrations in the database. A warning + doesn't prevent the command from executing. Default value is ``False``. + .. attribute:: BaseCommand.requires_system_checks A boolean; if ``True``, the entire Django project will be checked for diff --git a/docs/releases/1.10.txt b/docs/releases/1.10.txt index c152c1dc780..db4f7d4632d 100644 --- a/docs/releases/1.10.txt +++ b/docs/releases/1.10.txt @@ -248,6 +248,12 @@ Management Commands * Added a warning to :djadmin:`dumpdata` if a proxy model is specified (which results in no output) without its concrete parent. +* The new :attr:`BaseCommand.requires_migrations_checks + ` attribute + may be set to ``True`` if you want your command to print a warning, like + :djadmin:`runserver` does, if the set of migrations on disk don't match the + migrations in the database. + Migrations ~~~~~~~~~~ diff --git a/tests/admin_scripts/tests.py b/tests/admin_scripts/tests.py index 60c3a68fd2d..8f2ba6cab19 100644 --- a/tests/admin_scripts/tests.py +++ b/tests/admin_scripts/tests.py @@ -1367,7 +1367,7 @@ class ManageRunserver(AdminScriptTestCase): Ensure runserver.check_migrations doesn't choke on empty DATABASES. """ tested_connections = ConnectionHandler({}) - with mock.patch('django.core.management.commands.runserver.connections', new=tested_connections): + with mock.patch('django.core.management.base.connections', new=tested_connections): self.cmd.check_migrations() def test_readonly_database(self): diff --git a/tests/user_commands/tests.py b/tests/user_commands/tests.py index 2c19339ddc1..9ce13990404 100644 --- a/tests/user_commands/tests.py +++ b/tests/user_commands/tests.py @@ -7,12 +7,14 @@ from django.core import management from django.core.management import BaseCommand, CommandError, find_commands from django.core.management.utils import find_command, popen_wrapper from django.db import connection -from django.test import SimpleTestCase, override_settings +from django.test import SimpleTestCase, mock, override_settings from django.test.utils import captured_stderr, extend_sys_path from django.utils import translation from django.utils._os import upath from django.utils.six import StringIO +from .management.commands import dance + # A minimal set of apps to avoid system checks running on all apps. @override_settings( @@ -161,6 +163,18 @@ class CommandTests(SimpleTestCase): finally: BaseCommand.check = saved_check + def test_check_migrations(self): + requires_migrations_checks = dance.Command.requires_migrations_checks + try: + with mock.patch.object(BaseCommand, 'check_migrations') as check_migrations: + management.call_command('dance', verbosity=0) + self.assertFalse(check_migrations.called) + dance.Command.requires_migrations_checks = True + management.call_command('dance', verbosity=0) + self.assertTrue(check_migrations.called) + finally: + dance.Command.requires_migrations_checks = requires_migrations_checks + class CommandRunTests(AdminScriptTestCase): """