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 [email protected]. To post to this group, send email to [email protected]. 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.
