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

2014-11-10 Thread Tim Heap
The preference seems to be for option 3, adding a new flag --exit. I have 
implemented this and updated the pull request at 
https://github.com/django/django/pull/3441.

On Wednesday, October 29, 2014 12:22:42 PM UTC+11, Tim Heap wrote:
>
> Hi all,
>
> I have created a ticket for this (
> https://code.djangoproject.com/ticket/23728) but I would like some input 
> before I work on it. I will copy the content of the ticket below for ease 
> of reading:
>
> 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, as 4 could be emulated using -ev0.
>
> I will implement this change using 2, unless other people have opinions on 
> the matter.
>
> Regards,
> Tim
>

-- 
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/9e2ab711-1e4c-4458-a9c5-973ad21f6c0e%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


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

2014-10-31 Thread Luke Plant
My preference for this would be option 3., on the grounds that:

* I'd rule out 1 because of the slight backwards incompatibility (which
can affect people - I remember a deploy script failing because of a
changed exit code for a Mercurial command). In particular, currently
there are other conditions when you get a non-zero exit code - for the
case of some crash due to incorrect setup or other runtime error. This
is a distinct case from  "this command errored because there were no
migrations to make".

* I'd rule out 2 because it really makes --dry-run behave differently
from normal, which breaks expectations

Basically, having a flag like "--exit" is nice and explicit.

Luke

On 29/10/14 01:22, Tim Heap wrote:
> Hi all,
> 
> I have created a ticket for this
> (https://code.djangoproject.com/ticket/23728) but I would like some
> input before I work on it. I will copy the content of the ticket below
> for ease of reading:
> 
> 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, as 4 could be emulated using -ev0.
> 
> I will implement this change using 2, unless other people have opinions
> on the matter.
> 
> Regards,
> Tim
> 
> -- 
> 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/92203dcb-a775-4c17-a831-97d01ce8af3c%40googlegroups.com
> .
> For more options, visit https://groups.google.com/d/optout.


-- 
"I was sad because I had no shoes, until I met a man who had no
feet. So I said, "Got any shoes you're not using?"  (Steven Wright)

Luke Plant || http://lukeplant.me.uk/

-- 
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/

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

2014-10-30 Thread Shai Berger
On Thursday 30 October 2014 15:22:01 Dan Poirier wrote:
> Wouldn't enabling this by default cause a problem for any automated
> deploys?

The discussion is about the makemigrations command, not migrate. The OP wants 
to use it to find automatically if there are model changes not covered by 
migrations. While it is valid to do so in deployment, I don't think it is very 
common practice.

Shai.

-- 
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/7745956.FGeGuRyWyG%40deblack.
For more options, visit https://groups.google.com/d/optout.


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

2014-10-30 Thread Dan Poirier
Wouldn't enabling this by default cause a problem for any automated
deploys?  Ours typically run the migrate command on every deploy, just
in case there are new migrations, and any command that returns a non-0
exit status is going to look like a deploy failure.

Having the behavior change with a new command line option would be just
fine.

Dan

On Wed. 2014-10-29 at 03:44 AM EDT, Marc Tamlyn  wrote:

> I agree number 1 is fine. For most general interaction with the
> command users won't notice.
>
> Marc
>
> On 29 Oct 2014 02:04, "Andrew Godwin" <
> and...@aeracode.org> wrote:
>
> I'm actually fine with option 1 - always exiting with a status
> code if no migrations are found. Since the status code does
> nothing useful at the moment, I don't see any backwards
> compatibility issues, and as long as it's a suitably small patch,
> it should be fine.
>
> (As for being surprising compared to grep, there are many other
> commands with different exit codes one could draw parallels to;
> I'm not sure being consistent with a very different utility is a
> worthwhile cause).
>
> Andrew
>
> On Tue, Oct 28, 2014 at 6:22 PM, Tim Heap  o5...@public.gmane.org> wrote:
>
> Hi all,
>
> I have created a ticket for this (https://
> code.djangoproject.com/ticket/23728) but I would like some
> input before I work on it. I will copy the content of the
> ticket below for ease of reading:
>
> 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, as
> 4 could be emulated using -ev0.
>
> I will implement this change using 2, unless other people
> have opinions on the matter.
>
> Regards,
> Tim
>
> --
> 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+unsubscribe-/
> jypxa39uh5tlh3mboc...@public.gmane.org.
> To post to this group, send email to django-developers-/
> jypxa39uh5t

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

2014-10-29 Thread Tim Heap
If makemigrations exits with something other than 0, it will not have 
created any migrations to run anyway, so running the makemigrations & 
migrate pair here would not be affected. However, a useful one liner that I 
have been using when developing is:

./manage.py makemigrations && ./manage.py migrate && ./manage.py runserver

If the exit code was always used, chains like this would become much more 
awkward to use, which is why my preference is for option 2 or 3.

Making migrations and not applying them is useful if you want to review or 
edit the migrations before applying them - perhaps to split the migration 
in to two logical chunks for two different commits, or to append some 
manual migration code to the automatically generated migration. As part of 
continuous integration I also want to run:

./manage.py makemigrations --dry-run --exit || fail-build "Outstanding 
database changes with no migrations"

This way, the build fails if there are outstanding migrations. I am not 
interested in creating or running migrations in this instance, just 
checking that everything is up to date.

On Thursday, October 30, 2014 12:21:11 PM UTC+11, Daryl wrote:
>
> I'm +1 on being able to continue using the line 
> ./manage.py makemigrations && ./manage.py migrate 
>
> I can't see many (any?) situation where someone *wouldn't* run 
> makemigrations & migrate as one logical operation, whether by typing 
> the commands or running a script. 
> What would the workflow be where you would make migrations but not apply 
> them? 
>
> D 
>
>
>
> On 30 October 2014 12:29, Tim Heap > wrote: 
> > The backwards compatibility issue I was thinking of is for people using 
> > `makemigrations` in a script currently, expecting it to exit with 0 
> unless 
> > something very wrong has happened. For example, if `makemigrations` is 
> run 
> > in a bash script with `set -e`, and it does not find any migrations, 
> that 
> > script will then exit. Alternately, if someone is using a one-liner like 
> > `./manage.py makemigrations && ./manage.py migrate && another command` 
> the 
> > makemigrations command will cause the whole command to abort early, even 
> > though no 'error' as such has happened. Backwards compatibility aside, 
> this 
> > would be awkward to work around in scripts without simply ignoring the 
> exit 
> > code of `makemigrations` completely, which then ignores legitimate 
> errors. 
> > 
> > I have made a patch that calls `sys.exit(1)` when no changes are found, 
> and 
> > made a pull request at https://github.com/django/django/pull/3441 
> > 
> > Tim 
> > 
>

-- 
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/102bf2cd-f843-4dac-9374-6d1f5d20c59f%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


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

2014-10-29 Thread Daryl
I'm +1 on being able to continue using the line
./manage.py makemigrations && ./manage.py migrate

I can't see many (any?) situation where someone *wouldn't* run
makemigrations & migrate as one logical operation, whether by typing
the commands or running a script.
What would the workflow be where you would make migrations but not apply them?

D



On 30 October 2014 12:29, Tim Heap  wrote:
> The backwards compatibility issue I was thinking of is for people using
> `makemigrations` in a script currently, expecting it to exit with 0 unless
> something very wrong has happened. For example, if `makemigrations` is run
> in a bash script with `set -e`, and it does not find any migrations, that
> script will then exit. Alternately, if someone is using a one-liner like
> `./manage.py makemigrations && ./manage.py migrate && another command` the
> makemigrations command will cause the whole command to abort early, even
> though no 'error' as such has happened. Backwards compatibility aside, this
> would be awkward to work around in scripts without simply ignoring the exit
> code of `makemigrations` completely, which then ignores legitimate errors.
>
> I have made a patch that calls `sys.exit(1)` when no changes are found, and
> made a pull request at https://github.com/django/django/pull/3441
>
> Tim
>

-- 
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/CALzH9qsJ-qgPxGzmeMF3Lt9oXsn-eRk%3DUSNS%2BxZmbaKinMA2ww%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


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

2014-10-29 Thread Tim Heap
I also agree that users should opt-in to enabling the error, using option 2 
or 3, but I will implement what ever the popular option is.

My argument regarding "As the command is instructed to make migrations, not 
making any is the error case." was more about when to return the error: 
either when changes *are* found, or when changes are *not* found. As the 
command is attempting to make a migration, not finding any changes is the 
exceptional case, and should return an error code (if enabled via flags, or 
however it will eventually work).

On Thursday, October 30, 2014 10:30:50 AM UTC+11, Markus Holtermann wrote:
>
> I like the general idea, but I slightly disagree with "As the command is 
> instructed to make migrations, not making any is the error case.". Yes, 
> grep works that way but I thing this are two different use cases. I think 
> makemigrations on a project without any changes in the models' database 
> representation shouldn't exit with RC>0. I'd prefer to have the return code 
> either only set on dry-run or explicitly when using -e/--exit. I'm in favor 
> of 2. or 3.
>
> /Markus
>
> On Wednesday, October 29, 2014 8:44:31 AM UTC+1, Marc Tamlyn wrote:
>>
>> I agree number 1 is fine. For most general interaction with the command 
>> users won't notice.
>>
>> Marc
>> On 29 Oct 2014 02:04, "Andrew Godwin"  wrote:
>>
>>> I'm actually fine with option 1 - always exiting with a status code if 
>>> no migrations are found. Since the status code does nothing useful at the 
>>> moment, I don't see any backwards compatibility issues, and as long as it's 
>>> a suitably small patch, it should be fine.
>>>
>>> (As for being surprising compared to grep, there are many other commands 
>>> with different exit codes one could draw parallels to; I'm not sure being 
>>> consistent with a very different utility is a worthwhile cause).
>>>
>>> Andrew
>>>
>>> On Tue, Oct 28, 2014 at 6:22 PM, Tim Heap  wrote:
>>>
 Hi all,

 I have created a ticket for this (
 https://code.djangoproject.com/ticket/23728) but I would like some 
 input before I work on it. I will copy the content of the ticket below for 
 ease of reading:

 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, as 4 could be emulated using -ev0.

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

 Regards,
 Tim

 -- 

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

2014-10-29 Thread Markus Holtermann
I like the general idea, but I slightly disagree with "As the command is 
instructed to make migrations, not making any is the error case.". Yes, 
grep works that way but I thing this are two different use cases. I think 
makemigrations on a project without any changes in the models' database 
representation shouldn't exit with RC>0. I'd prefer to have the return code 
either only set on dry-run or explicitly when using -e/--exit. I'm in favor 
of 2. or 3.

/Markus

On Wednesday, October 29, 2014 8:44:31 AM UTC+1, Marc Tamlyn wrote:
>
> I agree number 1 is fine. For most general interaction with the command 
> users won't notice.
>
> Marc
> On 29 Oct 2014 02:04, "Andrew Godwin" > 
> wrote:
>
>> I'm actually fine with option 1 - always exiting with a status code if no 
>> migrations are found. Since the status code does nothing useful at the 
>> moment, I don't see any backwards compatibility issues, and as long as it's 
>> a suitably small patch, it should be fine.
>>
>> (As for being surprising compared to grep, there are many other commands 
>> with different exit codes one could draw parallels to; I'm not sure being 
>> consistent with a very different utility is a worthwhile cause).
>>
>> Andrew
>>
>> On Tue, Oct 28, 2014 at 6:22 PM, Tim Heap > 
>> wrote:
>>
>>> Hi all,
>>>
>>> I have created a ticket for this (
>>> https://code.djangoproject.com/ticket/23728) but I would like some 
>>> input before I work on it. I will copy the content of the ticket below for 
>>> ease of reading:
>>>
>>> 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, as 4 could be emulated using -ev0.
>>>
>>> I will implement this change using 2, unless other people have opinions 
>>> on the matter.
>>>
>>> Regards,
>>> Tim
>>>
>>> -- 
>>> 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-develop...@googlegroups.com .
>>> To post to this group, send email to django-d...@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/92203dcb-a775-4c17-a831-97d01ce8af3c%40googlegroups.com
>>>  
>>> 
>>> .

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

2014-10-29 Thread Tim Heap
The backwards compatibility issue I was thinking of is for people using 
`makemigrations` in a script currently, expecting it to exit with 0 unless 
something very wrong has happened. For example, if `makemigrations` is run 
in a bash script with `set -e`, and it does not find any migrations, that 
script will then exit. Alternately, if someone is using a one-liner like 
`./manage.py makemigrations && ./manage.py migrate && another command` the 
makemigrations command will cause the whole command to abort early, even 
though no 'error' as such has happened. Backwards compatibility aside, this 
would be awkward to work around in scripts without simply ignoring the exit 
code of `makemigrations` completely, which then ignores legitimate errors.

I have made a patch that calls `sys.exit(1)` when no changes are found, and 
made a pull request at https://github.com/django/django/pull/3441

Tim

On Wednesday, October 29, 2014 1:04:31 PM UTC+11, Andrew Godwin wrote:
>
> I'm actually fine with option 1 - always exiting with a status code if no 
> migrations are found. Since the status code does nothing useful at the 
> moment, I don't see any backwards compatibility issues, and as long as it's 
> a suitably small patch, it should be fine.
>
> (As for being surprising compared to grep, there are many other commands 
> with different exit codes one could draw parallels to; I'm not sure being 
> consistent with a very different utility is a worthwhile cause).
>
> Andrew
>
> On Tue, Oct 28, 2014 at 6:22 PM, Tim Heap > 
> wrote:
>
>> Hi all,
>>
>> I have created a ticket for this (
>> https://code.djangoproject.com/ticket/23728) but I would like some input 
>> before I work on it. I will copy the content of the ticket below for ease 
>> of reading:
>>
>> 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, as 4 could be emulated using -ev0.
>>
>> I will implement this change using 2, unless other people have opinions 
>> on the matter.
>>
>> Regards,
>> Tim
>>
>> -- 
>> 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-develop...@googlegroups.com .
>> To post to this group, send email to django-d...@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/92203dcb-a775-4c17-a831-97d01ce8af3c%

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

2014-10-29 Thread Marc Tamlyn
I agree number 1 is fine. For most general interaction with the command
users won't notice.

Marc
On 29 Oct 2014 02:04, "Andrew Godwin"  wrote:

> I'm actually fine with option 1 - always exiting with a status code if no
> migrations are found. Since the status code does nothing useful at the
> moment, I don't see any backwards compatibility issues, and as long as it's
> a suitably small patch, it should be fine.
>
> (As for being surprising compared to grep, there are many other commands
> with different exit codes one could draw parallels to; I'm not sure being
> consistent with a very different utility is a worthwhile cause).
>
> Andrew
>
> On Tue, Oct 28, 2014 at 6:22 PM, Tim Heap  wrote:
>
>> Hi all,
>>
>> I have created a ticket for this (
>> https://code.djangoproject.com/ticket/23728) but I would like some input
>> before I work on it. I will copy the content of the ticket below for ease
>> of reading:
>>
>> 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, as 4 could be emulated using -ev0.
>>
>> I will implement this change using 2, unless other people have opinions
>> on the matter.
>>
>> Regards,
>> Tim
>>
>> --
>> 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/92203dcb-a775-4c17-a831-97d01ce8af3c%40googlegroups.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/CAFwN1up96C8Daxn87Mt1y-TY%3DvGP_%3DTGaBRH6NZ9s5AQUu%3DL0A%40mail.gmail.com
> 

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

2014-10-28 Thread Andrew Godwin
I'm actually fine with option 1 - always exiting with a status code if no
migrations are found. Since the status code does nothing useful at the
moment, I don't see any backwards compatibility issues, and as long as it's
a suitably small patch, it should be fine.

(As for being surprising compared to grep, there are many other commands
with different exit codes one could draw parallels to; I'm not sure being
consistent with a very different utility is a worthwhile cause).

Andrew

On Tue, Oct 28, 2014 at 6:22 PM, Tim Heap  wrote:

> Hi all,
>
> I have created a ticket for this (
> https://code.djangoproject.com/ticket/23728) but I would like some input
> before I work on it. I will copy the content of the ticket below for ease
> of reading:
>
> 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, as 4 could be emulated using -ev0.
>
> I will implement this change using 2, unless other people have opinions on
> the matter.
>
> Regards,
> Tim
>
> --
> 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/92203dcb-a775-4c17-a831-97d01ce8af3c%40googlegroups.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/CAFwN1up96C8Daxn87Mt1y-TY%3DvGP_%3DTGaBRH6NZ9s5AQUu%3DL0A%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


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

2014-10-28 Thread Tim Heap
Hi all,

I have created a ticket for this (
https://code.djangoproject.com/ticket/23728) but I would like some input 
before I work on it. I will copy the content of the ticket below for ease 
of reading:

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, as 4 could be emulated using -ev0.

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

Regards,
Tim

-- 
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/92203dcb-a775-4c17-a831-97d01ce8af3c%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.