`makemigrations --check` output regression in 4.2

Django Internals ORM
1

Previously, the following invocation would check for pending migrations, output a summary (without writing to disk), and exit with code 1 if any were pending:

$ ./manage.py makemigrations --dry-run --check
Migrations for 'example':
  example/migrations/0002_test.py
    - Alter field name on author

$ echo $?
1

I’ve used this for years, adding it to projects I’ve worked on and recommending it in my book Boost Your Django DX.

Ticket #34051 changed --check so that it wouldn’t write migrations to disk, whether or not --dry-run was passed. I like that simplification.

But the change also removed the output listing the pending migrations. That same invocation is now silent from Django 4.2 (whether or not --dry-run is passed):

$ ./manage.py makemigrations --dry-run --check

$ echo $?
1

@shangxiao spotted this regression and opened #34457, and yesterday I clumsily opened a duplicate, #34935. #34457 was wontfixed, with the proposal from @felixxm that projects can now use a double-run approach to check for pending migrations and display them:

RESULT=`./manage.py makemigrations --check`
if [ "$RESULT" -gt 0 ]; then
    ./manage.py makemigrations --dry-run
fi;
exit $RESULT

I don’t think this is satisfactory. The same invocation no longer shows the pending migrations so I think it should be treated as a regression. Existing projects will need to update if they still want the output. Changing the command to restore the output costs little (see PR) and it can always be silenced with the universal option -v 0.

Furthermore, it’s inconsistent with optimizemigration --check, which still outputs what it would do:

$ ./manage.py optimizemigration --check example 0001
Optimizing from 3 operations to 1 operations.

$ echo $?
1

Also, it doesn’t seem like the output removal was exactly intentional. It was not part of the proposal that @claudep accepted in the original ticket #34051 , and was not tested before.

On my duplicated ticket #34935, @jacobtylerwalls asked me to seek consensus on this issue. Can I get some feedback on the proposal to restore the output?

3 Likes
2

+1 from me. I suspect that when I was working on #34051 I was thinking that --dry-run would still allow logging if desired, but you’re correct that was missed/not tested.

1 Like
3

Naturally I’m +1 :green_heart:

The same invocation no longer shows the pending migrations so I think it should be treated as a regression.

That’s a nice point that I didn’t pick up myself.

My PR was quite similar to yours: Fixed #34457 -- Print the changes makemigrations will make with --check by shangxiao · Pull Request #16723 · django/django · GitHub I don’t remember what I was doing on line 350 :thinking:

4

I’m also +1 for printing the migration details that would have be printed if --dry-run would have been run. My rationale is twofold:

  1. I don’t agree that the current behavior is “consistent with other uses of --check”, since other commands providing --check, do not provide --dry-run. So: we could argue that “every other command that provides both --check and --dry-run have --check implying --dry-run” :clown_face: (since the set of commands for this sentence is empty, the assertion is True). Seriously, my point is that we can make this change without introducing an inconsistency. If anything we would be defining a precedent for future commands that provide both flags.

  2. If running makemigrations --check when there are no pending migrations, the output does print the same message as if --dry-run was indeed passed:

    $ python -Wall manage.py makemigrations --check
No changes detected
$ echo $?
0

In summary, my point 1 “proves” (:smile:) that doing this would not add any inconsistency with other commands, and my point 2 shows how we would be instead fixing an existing inconsistency within the makemigrations command itself.

2 Likes
5

Thanks for the gut checks, all! I’ll go accept the ticket.