Opened 10 years ago

Closed 10 years ago

Last modified 9 years ago

#23728 closed New feature (fixed)

sys.exit(1) from makemigrations if no changes found

Reported by: Tim Heap Owned by: nobody
Component: Core (Management commands) Version: dev
Severity: Normal Keywords: migrations
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Tim Heap)

It would be very useful for continuous deployment, testing, commit hooks, and other applications if django-admin makemigrations signaled via an exit code if any migrations were found. Commits in projects could be rejected if migrations were outstanding, continuous deployment systems could fail the build on outstanding migrations, and potentially other uses. No more would hasty commits break things when developers forgot to make migrations!

Changes to the code to make this happen are easy enough, but I am unsure how the command should behave. The grep unix utility is a example to copy. Under normal operation, grep always exits 0 unless an error happens, regardless of whether it found any matches. Invoking grep with the -q/--quiet flag causes grep to be silent, not printing anything, as well as exiting 0 if matches are found and 1 if nothing is found.

I am proposing django-admin makemigrations should exit with 1 (or anything non-zero) if no migrations to make were found, or exit 0 if migrations to make were found. As the command is instructed to make migrations, not making any is the error case.

I am unsure how this new functionality should be selected by the user when invoking makemigrations. The options I see are:

  1. Enable this always. This is very simple to implement and easy to understand. Good unixy tools use error codes extensively to signal errors. This may be surprising behaviour when compared to grep though, and breaks backwards compatibility in a minor way.
  2. Enable this when the --dry-run flag is enabled. Now this flag can be used to check for migrations that need to be created both visually via the printed text, and composed in shell commands.
  3. Add a new flag -e/--exit (or similar). The sole purpose of this flag would be to exit with 1 when no migrations were found. This could be combined with --dry-run to just check for migrations that need to be made.
  4. Add a new flag -q/--quiet that copies the behaviour of greps -q/--quiet flag: silences output and exits with 1 when no migrations were found. This duplicates functionality though, as logging can be silenced using -v0 already.

My personal preference is for option 2. I was surprised when enabling --dry-run did not signal its result via the exit code. 3 would be the cleanest and most composable option.

I will implement this change using 2, unless other people have opinions on the matter.

Change History (7)

comment:1 by Tim Heap, 10 years ago

Description: modified (diff)

comment:2 by Tim Graham, 10 years ago

Triage Stage: UnreviewedAccepted
Version: 1.7master

comment:3 by Tim Heap, 10 years ago

A pull request has been created for this ticket: https://github.com/django/django/pull/3441

comment:4 by Tim Graham, 10 years ago

Has patch: set
Needs documentation: set

comment:5 by Tim Graham <timograham@…>, 10 years ago

Resolution: fixed
Status: newclosed

In deb607648ed7d484a90474b6da48642b4f39bca7:

Fixed #23728 -- Added the --exit option to makemigrations.

If no changes that need migrations are found, makemigrations --exit
exits with error code 1.

comment:6 by Martin Geisler, 9 years ago

I found this feature today when I looked for a way to check if someone had forgotten to generate and commit all migrations. That is, the error case is when there are pending migrations — when makemigrations --dry-run --exit returns 0.

So I need to negate the return value. That is certainly doable, but when using Tox, it means depending on an external command (the shell):

[testenv:migrations]
deps =
    -rrequirements.txt
commands =
    sh -c '! python manage.py makemigrations --dry-run --exit'
whitelist_externals =
    sh

My project is Linux-only, so the above should work for me. However, a project that wants to support Windows would need a work-around.

Maybe makemigrations could grow a --check flag that would enable --dry-run and exit with 1 if there are pending migrations and 0 otherwise?

comment:7 by Martin Geisler, 9 years ago

The work-around turned out to be easy! Drop this into a your_app/management/commands/checkmigrations.py file:

from django.core.management.commands import makemigrations


class Command(makemigrations.Command):
    help = 'Check that there are no outstanding migrations.'

    def add_arguments(self, parser):
        parser.add_argument('args', metavar='app_label', nargs='*',
                            help=('Specify the app label(s) to check '
                                  'migrations for.'))

    def handle(self, *args, **options):
        options['dry_run'] = True
        options['exit_code'] = True
        try:
            super().handle(*args, **options)
            # The handle method doesn't call sys.exit when it is
            # successful, so we do it here.
            raise SystemExit(0)
        except SystemExit as exc:
            # Swap exit code 0 and 1.
            if exc.code == 0:
                if self.verbosity >= 1:
                    msg = ("Outstanding migrations detected, "
                           "please run makemigrations.")
                    self.stdout.write(self.style.ERROR(msg))
                raise SystemExit(1)
            if exc.code == 1:
                raise SystemExit(0)
            raise  # Let other exit codes propagate unchanged.
Note: See TracTickets for help on using tickets.
Back to Top