I see what you mean and the inherent problems with the design. However
fundamentally the command you are running is "make some migrations", and
the "I don't have any to make" step is clearly an error case, not a success
case. In particular it would be very wrong for the real "I want to make
some migrations" command to return a non-zero code when it does
successfully create some.

The only real solution I can see is a separate command, or something like
makemigrations --check.

On 20 October 2015 at 05:20, Jon Dufresne <jon.dufre...@gmail.com> wrote:

> Before posting this, I've read through the full thread "sys.exit(1)
> from makemigrations if no changes found" at
> <
> https://groups.google.com/d/topic/django-developers/I3M6B-wYYd8/discussion
> >.
>
> I fully agree with the spirit of the change. I already find the
> feature useful in CI. However, after using this feature on a CI
> server, I find the exit status backwards compared to typical commands.
> The makemigrations command returns status 0 to indicate CI failure
> (migrations missing) and 1 to indicate CI pass (continue to the next
> CI stage).
>
> Typically status 0 indicates pass and non-zero indicates failure. By
> following the typical exit status conventions, commands can explicitly
> return a non-zero status when detecting a failure or the Python
> runtime may return a non-zero status if something goes terribly wrong;
> such as an unhandled exception.
>
> Someone that is accustomed to typical exit status conventions might
> naively use the makemigrations command:
>
> ./manage.py makemigrations --dry-run --exit
>
> The expectation: the next stage should continue if there are no new
> migrations (the developer did everything they were supposed to do and
> included migrations). However, the above command will return status 1,
> not 0, if there are no new migrations.
>
> OK, we can test for that. Maybe change the command to:
>
> ! ./manage.py makemigrations --dry-run --exit
>
> That is, interpret the exit status opposite of what one would
> typically expect. Immediately, this looks backwards compared to
> typical shell scripting. But what happened to the "terribly wrong"
> scenario? For example, what if a developer mistakenly added an
> incorrect setting that caused an ImproperlyConfigured error? If this
> were to happen, I would want the above command to fail and stop the CI
> pipeline.
>
> So maybe the next step would be to check explicitly for exit code 1.
>
> ./manage.py makemigrations --dry-run --exit; test $? -eq 1
>
> Now it looks like we're hacking around something.
>
> Additionally, Python exits with status 1 for unhandled exceptions. So
> the above command would still pass the CI stage for an unhandled
> exception. Further Django's BaseCommand.run_from_argv() also exits
> with status code 1 on CommandError, so again, it would pass the CI
> stage for anything that triggers this sort of error.
>
> It seems exit status 1 is overloaded to mean "all migrations are
> accounted for, continue to the next stage of CI", and "something went
> terribly wrong".
>
> This is why I feel the exit status is backwards from what is typically
> expected.
>
> I would like to suggest we find a better way to interface with CI
> servers. That is return 0 when there are no migrations (continue to
> the next stage) and non-zero for both "migrations missing" and
> "something went terribly wrong".
>
> I suggest maybe adding a system check for missing migrations. The
> check could report an error, when they are missing.  The check
> framework seems like a natural command to be used by CI servers
> anyway, so this seems like a good place. The missing migration
> detection already exists, so the same code could be leveraged for this
> check.
>
> I'm also open to other suggestions on creating a more convention exit
> status.
>
> If there is agreement and the proposal sounds good, I can follow
> through with a ticket and code changes.
>
> Cheers,
> Jon
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers  (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> Visit this group at http://groups.google.com/group/django-developers.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/CADhq2b5YDr-HB5sUdwKK-K2awQZk7qUhJJdaU%2B4SH_6nx9x%3D5w%40mail.gmail.com
> .
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAMwjO1G32WwtE2YXaqmGgpwvSGgx6_xOeEBGpAyr_J%2B5PcpEXg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to