Re: ./manage.py settings

['1337 Shadow Hacker' via Django developers (Contributions to Django itself)]

It wasn't discussed indeed, nice command, thank you !

-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/2LiWpUB0mNim6QOo9C4_1EUMeOFkZvE3zzZymvZt2gw2YJgW7u-s6G1VN32S5_3fqLPG2AaIdxONU3qTvAwxhVY5QH1VZ0RKKM1eBZStC48%3D%40protonmail.com.

Re: Defaulting to use BigAutoField in models

[Tom Forbes]

> I think we should restrict the setting between normal and big auto fields 
> only. Allowing UUID's would be changing the type, with the potential for 
> havoc with code incompalities throughout django. It's also not possible to 
> migrate tables over to the new type.

That’s a really good point, I got a bit swept up with the idea to realise that 
it would break a lot of things if applied generally!

> The autodetector knows if a model is new. It could be that during one version 
> Django outputs BigAutoField for fields added in CreateModel only.

We could make every automatic PK become a BigAutoField but include some logic 
in the migrations framework to ignore changes between an auto_created AutoField 
-> BigAutoField? This would ignore all existing models in your migration 
history, but all completely new models would receive BigAutoField PKs from the 
start. Users who explicitly add a `BigAutoField` to a model with a previously 
created `AutoField` would see a migration created as the `auto_created` flag is 
not set. It would also play nicely with foreign keys which also need to be 8 
bytes.

This would take care of third party apps with migrations and not require a 
setting but the downside is that model state will be slightly different from 
the database. All models would have `BigAutoField`’s whereas the database would 
only have an `AutoField`. This feels slightly iffy to me even though I cannot 
think of a case where it would have any practical effect.

> On 11 Jun 2020, at 19:22, Adam Johnson  wrote:
> 
> Big +1 on solving this from me.
> 
> - The setting would take any dotted path to a class, or a single class name 
> for a build in field. This would potentially solve [3], and could be useful 
> to people who want to default to other fields like UUIDs (or a custom 
> BigAutoField) for whatever reason
> 
> I think we should restrict the setting between normal and big auto fields 
> only. Allowing UUID's would be changing the type, with the potential for 
> havoc with code incompalities throughout django. It's also not possible to 
> migrate tables over to the new type.
> 
> What do you think the solution is for third-party apps? They ship their own 
> migrations and can't really be tied to project state.
> 
> As Django migrations are derived from the current model state so there’s no 
> way I can think of to express “make this auto-generated field a BigAutoField 
> only if this model is new”.
> 
> The autodetector knows if a model is new. It could be that during one version 
> Django outputs BigAutoField for fields added in CreateModel only.
> 
> On Thu, 11 Jun 2020 at 16:28, Tom Forbes  > wrote:
> I’d like to re-propose switching Django to use BigAutoField’s rather than the 
> current AutoField. This has been proposed[1] before (and a MR made[2]) but it 
> was closed due to implementation issues and not much else has happened since 
> then.
> 
> As many of you are aware the max value a standard AutoField can hold is 
> 2,147,483,647 (2.1 billion) which sounds like more than you can ever need. 
> But it’s often not, and you only find out at the worst possible time - out of 
> the blue at night and during a period of rapid growth. The process for fixing 
> this issue also becomes a lot harder as your data grows - when you’ve hit the 
> limit you’re looking at a multi-hour `ALTER TABLE` on Postgres during which 
> writes and reads are blocked, or a risky operation to create a new table with 
> the correct primary key and copy old data over in batches. Basically if 
> you’ve experienced this before you wouldn’t wish it on your worst enemy.
> 
> I’m proposing that we add a `MODELS_PRIMARY_KEY` (name improvements welcome!) 
> setting that _defaults_ to `BigAutoField`, with prominent docs/release notes 
> saying that to preserve the existing behaviour this should be set to 
> `AutoField`. I think this the only realistic way we can implement this for 
> new projects in a way that ensures it will be used meaningfully and not 
> forgotten about until it’s too late.
> 
> Rails managed to do this migration somewhat painlessly due the big 
> differences between Rails and Django models. As Django migrations are derived 
> from the current model state so there’s no way I can think of to express 
> “make this auto-generated field a BigAutoField only if this model is new”. 
> The way I see it is that a global setting is very easy to toggle and there is 
> little chance of missing the large numbers of migrations that would be 
> generated during the Django update. Smaller applications could apply the 
> migrations with little issue and larger applications would be able to opt-out 
> (as well as be reminded that this is a problem they could face!).
> 
> Some specifics:
> - The setting would take any dotted path to a class, or a single class name 
> for a build in field. This would potentially solve [3], and could be useful 
> to people who want to default to other fields like UUIDs (or a c

Re: Defaulting to use BigAutoField in models

[Adam Johnson]

I haven't thought this through, but maybe it's possible to restrict the
change to inside AutoField, rather than create a new field class. If its
db_parameters() method / db_type() could receive enough context from the
migration history, it could potentially determine its own database type.
For those who want to fixed field sizes, BigAutoField and SmallAutoField
could be left, and MediumAutoField added.

(Whilst poking around I think I spotted a small cleanup:
https://code.djangoproject.com/ticket/31698 )

On Fri, 12 Jun 2020 at 11:40, Tom Forbes  wrote:

> I think we should restrict the setting between normal and big auto fields
> only. Allowing UUID's would be changing the type, with the potential for
> havoc with code incompalities throughout django. It's also not possible to
> migrate tables over to the new type.
>
>
> That’s a really good point, I got a bit swept up with the idea to realise
> that it would break a lot of things if applied generally!
>
> The autodetector knows if a model is new. It could be that during one
> version Django outputs BigAutoField for fields added in CreateModel only.
>
>
> We could make every automatic PK become a BigAutoField but include some
> logic in the migrations framework to *ignore* changes between an
> auto_created AutoField -> BigAutoField? This would ignore all existing
> models in your migration history, but all completely new models would
> receive BigAutoField PKs from the start. Users who explicitly add a
> `BigAutoField` to a model with a previously created `AutoField` would see a
> migration created as the `auto_created` flag is not set. It would also play
> nicely with foreign keys which also need to be 8 bytes.
>
> This would take care of third party apps with migrations and not require a
> setting but the downside is that model state will be slightly different
> from the database. All models would have `BigAutoField`’s whereas the
> database would only have an `AutoField`. This feels slightly iffy to me
> even though I cannot think of a case where it would have any practical
> effect.
>
> On 11 Jun 2020, at 19:22, Adam Johnson  wrote:
>
> Big +1 on solving this from me.
>
> - The setting would take any dotted path to a class, or a single class
>> name for a build in field. This would potentially solve [3], and could be
>> useful to people who want to default to other fields like UUIDs (or a
>> custom BigAutoField) for whatever reason
>>
>
> I think we should restrict the setting between normal and big auto fields
> only. Allowing UUID's would be changing the type, with the potential for
> havoc with code incompalities throughout django. It's also not possible to
> migrate tables over to the new type.
>
> What do you think the solution is for third-party apps? They ship their
> own migrations and can't really be tied to project state.
>
> As Django migrations are derived from the current model state so there’s
>> no way I can think of to express “make this auto-generated field a
>> BigAutoField only if this model is *new*”.
>>
>
> The autodetector knows if a model is new. It could be that during one
> version Django outputs BigAutoField for fields added in CreateModel only.
>
> On Thu, 11 Jun 2020 at 16:28, Tom Forbes  wrote:
>
>> I’d like to re-propose switching Django to use BigAutoField’s rather than
>> the current AutoField. This has been proposed[1] before (and a MR made[2])
>> but it was closed due to implementation issues and not much else has
>> happened since then.
>>
>> As many of you are aware the max value a standard AutoField can hold is
>> 2,147,483,647 (2.1 billion) which sounds like more than you can ever need.
>> But it’s often not, and you only find out at the worst possible time - out
>> of the blue at night and during a period of rapid growth. The process for
>> fixing this issue also becomes a lot harder as your data grows - when
>> you’ve hit the limit you’re looking at a multi-hour `ALTER TABLE` on
>> Postgres during which writes and reads are blocked, or a risky operation to
>> create a new table with the correct primary key and copy old data over in
>> batches. Basically if you’ve experienced this before you wouldn’t wish it
>> on your worst enemy.
>>
>> I’m proposing that we add a `MODELS_PRIMARY_KEY` (name improvements
>> welcome!) setting that _defaults_ to `BigAutoField`, with prominent
>> docs/release notes saying that to preserve the existing behaviour this
>> should be set to `AutoField`. I think this the only realistic way we can
>> implement this for new projects in a way that ensures it will be used
>> meaningfully and not forgotten about until it’s too late.
>>
>> Rails managed to do this migration somewhat painlessly due the big
>> differences between Rails and Django models. As Django migrations are
>> derived from the current model state so there’s no way I can think of to
>> express “make this auto-generated field a BigAutoField only if this model
>> is *new*”. The way I see it is that a global setting is very easy

Proposal - Warn user when creating or applying a delete migration?

[Jason Johns]

Should Django output a warning and/or require a prompt when a DeleteModel 
or RemoveField are to be executed when applying migrations?

Over at the pythondev slack group, a user wanted to rename a model to 
another name, and wasn't aware of  the `db_table` attribute in Model Meta.  
So a new model was created, the old one removed.  Then a migration was made 
and applied... only to delete the data on the staging environment.  This 
user was also unaware of the typical three step migration process 
recommended for removing a model or field, and made the statement that 
there should have been a prompt during migrate that data loss was a 
possibility.

This got me thinking.  Googling around seems to suggest this experience of 
running a delete migration is a somewhat common footgun because the 
implications are not as explicit as Python warnings and exceptions.  As a 
result, it might be beneficial for migrate to have some tweaks to attempt 
to reduce surprise.

How about this:

   1. Migrate checks if any executions of migrations.DeleteModel or 
   migrations.RemoveField are included in the migrations being applied
   2. If so, output a warning message about a table being dropped or column 
   being deleted and that data loss will occur
   3. Output a yes/no input prompt asking if the user has made a data 
   migration or is aware and acknowledges the risk


Is this something that would fit in with the overall philosophy of Django?  
To be honest, I'm undecided about this, but this may be my own biased 
feelings that this should be implicit in the developer's mind when 
considering removing columns and tables.  But then again, I have been 
burned by this in a toy project.

-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/e4af0410-7eab-4431-8c56-5094d9498b92o%40googlegroups.com.

Re: ./manage.py settings

[René Fleschenberg]

Hi,

Just a thought that came to my mind: It would be useful to have a
command that dumps the run-time settings, but automatically replaces
secrets with dummy values.

I think this should not be too hard to do for Django's own secret
settings, and maybe it can also be done for some known common third
party settings (AWS_SECRET_ACCESS_KEY, for example).

Even better would be to have a standardized way of marking settings as
secret, though I am not sure if this is feasible.

Regards,
René

On 6/12/20 5:09 AM, '1337 Shadow Hacker' via Django developers
(Contributions to Django itself) wrote:
> Hi all,
> 
> So, just on #django IRC channel there was a user trying to help another
> one, asking for some settings through ./manage.py shell etc ... A
> discussion that went kind of like "Print out your settings" "How would I
> print, I tried that, I'm in settings.py" "With ... print()" "but in the
> shell, __file__ is not defined" and so on, and 20 minutes later the user
> couldn't print his settings and left.
> 
> TBH I'm pretty fine because the users I support are on projects where I
> added djcli to the requirements, so I can always ask the to do the djcli
> setting command to print out a setting. It's very useful useful to me, I
> think Django should a command to dump runtime settings because that
> should make the life easier of Django users trying to support newbies in
> their own projects, where they don't have installed a custom command,
> it's pretty cheap to make and maintain and should save quite some
> frustration from both sides of the story.
> 
> What do you think ?
> 
> 
> -- 
> 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 view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/X2HkH0yasxu2XVxBuVkbTfdSHctL7y3pW9seQ8CHFcVOpmsiyN80JlDYH2g5F6IfPvEdN4zyG6vuxj2HEMJaXUSErMUM5IT70iLvkxsrc7U%3D%40protonmail.com
> .

-- 
René Fleschenberg

Rosastrasse 53, 45130 Essen, Germany
Phone: +49 1577 170 7363
https://fleschenberg.net
E-mail: r...@fleschenberg.net

-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/501b33d3-9191-69a0-01dd-5c875f048073%40fleschenberg.net.

Re: Proposal - Warn user when creating or applying a delete migration?

[Adam Johnson]

'makemigrations' is a code generator, not a "do it right all the time"
wizard. There are many situations where it doesn't do what the user
intended. Users are ultimately responsible for the contents of migration
files and should read them before applying them, and also use sqlmigrate to
see what the underlying SQL is.

I think our "workflow" section could make this clearer:
https://docs.djangoproject.com/en/3.0/topics/migrations/#workflow

I am unsure that adding warnings or errors will force users to pay more
attention. Often users run without warnings displayed.

On Fri, 12 Jun 2020 at 12:41, Jason Johns  wrote:

> Should Django output a warning and/or require a prompt when a DeleteModel
> or RemoveField are to be executed when applying migrations?
>
> Over at the pythondev slack group, a user wanted to rename a model to
> another name, and wasn't aware of  the `db_table` attribute in Model Meta.
> So a new model was created, the old one removed.  Then a migration was made
> and applied... only to delete the data on the staging environment.  This
> user was also unaware of the typical three step migration process
> recommended for removing a model or field, and made the statement that
> there should have been a prompt during migrate that data loss was a
> possibility.
>
> This got me thinking.  Googling around seems to suggest this experience of
> running a delete migration is a somewhat common footgun because the
> implications are not as explicit as Python warnings and exceptions.  As a
> result, it might be beneficial for migrate to have some tweaks to attempt
> to reduce surprise.
>
> How about this:
>
>1. Migrate checks if any executions of migrations.DeleteModel or
>migrations.RemoveField are included in the migrations being applied
>2. If so, output a warning message about a table being dropped or
>column being deleted and that data loss will occur
>3. Output a yes/no input prompt asking if the user has made a data
>migration or is aware and acknowledges the risk
>
>
> Is this something that would fit in with the overall philosophy of
> Django?  To be honest, I'm undecided about this, but this may be my own
> biased feelings that this should be implicit in the developer's mind when
> considering removing columns and tables.  But then again, I have been
> burned by this in a toy project.
>
> --
> 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 view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/e4af0410-7eab-4431-8c56-5094d9498b92o%40googlegroups.com
> 
> .
>


-- 
Adam

-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAMyDDM0ROmXKTaZ7fuw7cvXLLRk8AbrWw1EhcQMssOofpYGKGA%40mail.gmail.com.

Re: Proposal - Warn user when creating or applying a delete migration?

[Tom Forbes]

Fully agree, I would be against adding a y/n prompt. 

However we could make the `makemigrations` output more noticeable if it 
contains destructive operations? We could make the this line[1] output red text 
on destructive operations, yellow on maybe destructive (like altering a type), 
and green on safe operations?

1. 
https://github.com/django/django/blob/master/django/core/management/commands/makemigrations.py#L216
 


> On 12 Jun 2020, at 12:52, Adam Johnson  wrote:
> 
> 'makemigrations' is a code generator, not a "do it right all the time" 
> wizard. There are many situations where it doesn't do what the user intended. 
> Users are ultimately responsible for the contents of migration files and 
> should read them before applying them, and also use sqlmigrate to see what 
> the underlying SQL is.
> 
> I think our "workflow" section could make this clearer: 
> https://docs.djangoproject.com/en/3.0/topics/migrations/#workflow 
> 
> 
> I am unsure that adding warnings or errors will force users to pay more 
> attention. Often users run without warnings displayed.
> 
> On Fri, 12 Jun 2020 at 12:41, Jason Johns  > wrote:
> Should Django output a warning and/or require a prompt when a DeleteModel or 
> RemoveField are to be executed when applying migrations?
> 
> Over at the pythondev slack group, a user wanted to rename a model to another 
> name, and wasn't aware of  the `db_table` attribute in Model Meta.  So a new 
> model was created, the old one removed.  Then a migration was made and 
> applied... only to delete the data on the staging environment.  This user was 
> also unaware of the typical three step migration process recommended for 
> removing a model or field, and made the statement that there should have been 
> a prompt during migrate that data loss was a possibility.
> 
> This got me thinking.  Googling around seems to suggest this experience of 
> running a delete migration is a somewhat common footgun because the 
> implications are not as explicit as Python warnings and exceptions.  As a 
> result, it might be beneficial for migrate to have some tweaks to attempt to 
> reduce surprise.
> 
> How about this:
> Migrate checks if any executions of migrations.DeleteModel or 
> migrations.RemoveField are included in the migrations being applied
> If so, output a warning message about a table being dropped or column being 
> deleted and that data loss will occur
> Output a yes/no input prompt asking if the user has made a data migration or 
> is aware and acknowledges the risk
> 
> Is this something that would fit in with the overall philosophy of Django?  
> To be honest, I'm undecided about this, but this may be my own biased 
> feelings that this should be implicit in the developer's mind when 
> considering removing columns and tables.  But then again, I have been burned 
> by this in a toy project.
> 
> -- 
> 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 view this discussion on the web visit 
> https://groups.google.com/d/msgid/django-developers/e4af0410-7eab-4431-8c56-5094d9498b92o%40googlegroups.com
>  
> .
> 
> 
> -- 
> Adam
> 
> -- 
> 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 view this discussion on the web visit 
> https://groups.google.com/d/msgid/django-developers/CAMyDDM0ROmXKTaZ7fuw7cvXLLRk8AbrWw1EhcQMssOofpYGKGA%40mail.gmail.com
>  
> .

-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/69C34E37-ADE5-4B56-8439-5F8CC962017C%40tomforb.es.

Re: Proposal - Warn user when creating or applying a delete migration?

[René Fleschenberg]

Hi,

>  3. Output a yes/no input prompt asking if the user has made a data
> migration or is aware and acknowledges the risk

Migrations are often applied non-interactively during some kind of
automated deployment step. So this behaviour would have to be somehow
optional. Either it has to detect if it's being run interactively, or we
would have to add a command line switch or environment variable. In that
case there would be the question what the default should be here.
Defaulting to the prompt would be backwards-incompatible.

-- 
René Fleschenberg

-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/bde47e6c-98d2-2b37-ec87-9c9c0c3b4dd3%40fleschenberg.net.

Re: ./manage.py settings

[Jure Erznožnik]

There was a GSoC proposal 
 
this year for a SecretsManager that could conceivably make what you 
suggest possible. Wasn't accepted though.


Not sure if it's a security issue with beginners that require help 
setting up settings.py though. Should be easy enough to just paste the 
file (modified) somewhere.


LP,
Jure

On 12/06/2020 13:51, René Fleschenberg wrote:

Hi,

Just a thought that came to my mind: It would be useful to have a
command that dumps the run-time settings, but automatically replaces
secrets with dummy values.

I think this should not be too hard to do for Django's own secret
settings, and maybe it can also be done for some known common third
party settings (AWS_SECRET_ACCESS_KEY, for example).

Even better would be to have a standardized way of marking settings as
secret, though I am not sure if this is feasible.

Regards,
René

On 6/12/20 5:09 AM, '1337 Shadow Hacker' via Django developers
(Contributions to Django itself) wrote:

Hi all,

So, just on #django IRC channel there was a user trying to help another
one, asking for some settings through ./manage.py shell etc ... A
discussion that went kind of like "Print out your settings" "How would I
print, I tried that, I'm in settings.py" "With ... print()" "but in the
shell, __file__ is not defined" and so on, and 20 minutes later the user
couldn't print his settings and left.

TBH I'm pretty fine because the users I support are on projects where I
added djcli to the requirements, so I can always ask the to do the djcli
setting command to print out a setting. It's very useful useful to me, I
think Django should a command to dump runtime settings because that
should make the life easier of Django users trying to support newbies in
their own projects, where they don't have installed a custom command,
it's pretty cheap to make and maintain and should save quite some
frustration from both sides of the story.

What do you think ?


--
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 view this discussion on the web visit
https://groups.google.com/d/msgid/django-developers/X2HkH0yasxu2XVxBuVkbTfdSHctL7y3pW9seQ8CHFcVOpmsiyN80JlDYH2g5F6IfPvEdN4zyG6vuxj2HEMJaXUSErMUM5IT70iLvkxsrc7U%3D%40protonmail.com
.


--
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/4370f935-9d2d-283d-3156-14c878fc1c93%40gmail.com.

Re: Proposal - Warn user when creating or applying a delete migration?

[Adam Johnson]

That output change would be useful, but not accessible. Perhaps we should
also use "+" for creative and "-" for destructive operations?

We could also open the generated migration file in the user's $EDITOR, if
the terminal is interactive. This would be a nice usability improvement for
use of --empty or other situations where the user knows they need to edit
the migration too.

On Fri, 12 Jun 2020 at 12:56, Tom Forbes  wrote:

> Fully agree, I would be against adding a y/n prompt.
>
> However we could make the `makemigrations` output more noticeable if it
> contains destructive operations? We could make the this line[1] output red
> text on destructive operations, yellow on maybe destructive (like altering
> a type), and green on safe operations?
>
> 1.
> https://github.com/django/django/blob/master/django/core/management/commands/makemigrations.py#L216
>
> On 12 Jun 2020, at 12:52, Adam Johnson  wrote:
>
> 'makemigrations' is a code generator, not a "do it right all the time"
> wizard. There are many situations where it doesn't do what the user
> intended. Users are ultimately responsible for the contents of migration
> files and should read them before applying them, and also use sqlmigrate to
> see what the underlying SQL is.
>
> I think our "workflow" section could make this clearer:
> https://docs.djangoproject.com/en/3.0/topics/migrations/#workflow
>
> I am unsure that adding warnings or errors will force users to pay more
> attention. Often users run without warnings displayed.
>
> On Fri, 12 Jun 2020 at 12:41, Jason Johns  wrote:
>
>> Should Django output a warning and/or require a prompt when a DeleteModel
>> or RemoveField are to be executed when applying migrations?
>>
>> Over at the pythondev slack group, a user wanted to rename a model to
>> another name, and wasn't aware of  the `db_table` attribute in Model Meta.
>> So a new model was created, the old one removed.  Then a migration was made
>> and applied... only to delete the data on the staging environment.  This
>> user was also unaware of the typical three step migration process
>> recommended for removing a model or field, and made the statement that
>> there should have been a prompt during migrate that data loss was a
>> possibility.
>>
>> This got me thinking.  Googling around seems to suggest this experience
>> of running a delete migration is a somewhat common footgun because the
>> implications are not as explicit as Python warnings and exceptions.  As a
>> result, it might be beneficial for migrate to have some tweaks to attempt
>> to reduce surprise.
>>
>> How about this:
>>
>>1. Migrate checks if any executions of migrations.DeleteModel or
>>migrations.RemoveField are included in the migrations being applied
>>2. If so, output a warning message about a table being dropped or
>>column being deleted and that data loss will occur
>>3. Output a yes/no input prompt asking if the user has made a data
>>migration or is aware and acknowledges the risk
>>
>>
>> Is this something that would fit in with the overall philosophy of
>> Django?  To be honest, I'm undecided about this, but this may be my own
>> biased feelings that this should be implicit in the developer's mind when
>> considering removing columns and tables.  But then again, I have been
>> burned by this in a toy project.
>>
>> --
>> 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 view this discussion on the web visit
>> https://groups.google.com/d/msgid/django-developers/e4af0410-7eab-4431-8c56-5094d9498b92o%40googlegroups.com
>> 
>> .
>>
>
>
> --
> Adam
>
> --
> 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 view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/CAMyDDM0ROmXKTaZ7fuw7cvXLLRk8AbrWw1EhcQMssOofpYGKGA%40mail.gmail.com
> 
> .
>
>
> --
> 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 view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/69C34E37-ADE5-4B56-8439-5F8CC962017C%40tomforb.es
> 

Re: Proposal - Warn user when creating or applying a delete migration?

[Tom Forbes]

I think opening a migration file in the users $EDITOR could be a good flag to 
add, but I would be wary of making it a default behaviour. I can see it being 
useful in some situations but annoying in others.

I made a ticket for changing the output: 
https://code.djangoproject.com/ticket/31700

> On 12 Jun 2020, at 13:01, Adam Johnson  wrote:
> 
> That output change would be useful, but not accessible. Perhaps we should 
> also use "+" for creative and "-" for destructive operations?
> 
> We could also open the generated migration file in the user's $EDITOR, if the 
> terminal is interactive. This would be a nice usability improvement for use 
> of --empty or other situations where the user knows they need to edit the 
> migration too.
> 
> On Fri, 12 Jun 2020 at 12:56, Tom Forbes  > wrote:
> Fully agree, I would be against adding a y/n prompt. 
> 
> However we could make the `makemigrations` output more noticeable if it 
> contains destructive operations? We could make the this line[1] output red 
> text on destructive operations, yellow on maybe destructive (like altering a 
> type), and green on safe operations?
> 
> 1. 
> https://github.com/django/django/blob/master/django/core/management/commands/makemigrations.py#L216
>  
> 
> 
>> On 12 Jun 2020, at 12:52, Adam Johnson > > wrote:
>> 
>> 'makemigrations' is a code generator, not a "do it right all the time" 
>> wizard. There are many situations where it doesn't do what the user 
>> intended. Users are ultimately responsible for the contents of migration 
>> files and should read them before applying them, and also use sqlmigrate to 
>> see what the underlying SQL is.
>> 
>> I think our "workflow" section could make this clearer: 
>> https://docs.djangoproject.com/en/3.0/topics/migrations/#workflow 
>> 
>> 
>> I am unsure that adding warnings or errors will force users to pay more 
>> attention. Often users run without warnings displayed.
>> 
>> On Fri, 12 Jun 2020 at 12:41, Jason Johns > > wrote:
>> Should Django output a warning and/or require a prompt when a DeleteModel or 
>> RemoveField are to be executed when applying migrations?
>> 
>> Over at the pythondev slack group, a user wanted to rename a model to 
>> another name, and wasn't aware of  the `db_table` attribute in Model Meta.  
>> So a new model was created, the old one removed.  Then a migration was made 
>> and applied... only to delete the data on the staging environment.  This 
>> user was also unaware of the typical three step migration process 
>> recommended for removing a model or field, and made the statement that there 
>> should have been a prompt during migrate that data loss was a possibility.
>> 
>> This got me thinking.  Googling around seems to suggest this experience of 
>> running a delete migration is a somewhat common footgun because the 
>> implications are not as explicit as Python warnings and exceptions.  As a 
>> result, it might be beneficial for migrate to have some tweaks to attempt to 
>> reduce surprise.
>> 
>> How about this:
>> Migrate checks if any executions of migrations.DeleteModel or 
>> migrations.RemoveField are included in the migrations being applied
>> If so, output a warning message about a table being dropped or column being 
>> deleted and that data loss will occur
>> Output a yes/no input prompt asking if the user has made a data migration or 
>> is aware and acknowledges the risk
>> 
>> Is this something that would fit in with the overall philosophy of Django?  
>> To be honest, I'm undecided about this, but this may be my own biased 
>> feelings that this should be implicit in the developer's mind when 
>> considering removing columns and tables.  But then again, I have been burned 
>> by this in a toy project.
>> 
>> -- 
>> 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 view this discussion on the web visit 
>> https://groups.google.com/d/msgid/django-developers/e4af0410-7eab-4431-8c56-5094d9498b92o%40googlegroups.com
>>  
>> .
>> 
>> 
>> -- 
>> Adam
>> 
>> -- 
>> 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 
>> 

Re: Proposal - Warn user when creating or applying a delete migration?

[Jason Johns]

I appreciate the feedback!  

I do agree with you, Adam, that this may not be something that would 
strictly be Django's responsibility, but I also feel that if a thing is 
causing a number of footguns, regardless if its primarily the user's fault, 
Django is getting the blame.  I do like your suggestion to Tom's proposal, 
especially if its combined with shell colors.

Rene makes a very good point, and this would primarily be useful on the 
dev's machine when applying migrations after changes, and before they hit 
staging/prod.

thank you :-)

-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/b55252c8-8735-4d0b-a2e2-0e9d897226cfo%40googlegroups.com.

Re: Add verbosity option to manage.py checks that outputs which checks are ran

[Gordon]

The underlying problem that I want to solve is a way to fail a CI job if 
checks for an app don't run.  The only approach I currently see that would 
accomplish that is by using a custom tag for every project that could fail 
with the invalid tag error - but this feels wrong.

Would you be in favor of a flag for the check command that errors if checks 
aren't run from a particular namespace?

Like `python manage.py check --require-checks foo` which would exit with a 
non-zero exit code if zero checks for `foo` are run?



On Wednesday, June 10, 2020 at 6:40:16 PM UTC-4, Adam Johnson wrote:
>
> Perhaps there could be better on guidance in the documentation about 
> registering custom checks. I see nothing in the "writing your own checks" 
> guide really describes a recommended code structure. We'd accept a docs PR 
> there I'm sure.
>
> I normally have a checks submodule in my app and register the checks in 
> AppConfig.ready(). For example: 
> https://github.com/adamchainz/django-mysql/blob/2cf93c771f16011bc754a61e5aa95be4871f0363/src/django_mysql/apps.py#L17
>  
> . I don't like the decorator form since that's an import side effect and as 
> you've discovered it's not always easy to ensure your module gets imported.
>
> On Wed, 10 Jun 2020 at 21:16, Gordon > 
> wrote:
>
>> Particular case that spurred this - reusable app had checks to make sure 
>> urls were reversible.  Checks weren't imported.  Checks command passes but 
>> doesn't run the apps checks.  There is no indication that the checks were 
>> not run so you have to assume they did and were successful.
>>
>> The app checks were tested for function.  Since this imports the checks 
>> they would show as registered using your `get_checks()` test, no?  Seems 
>> kind of tricky...
>>
>> Since the checks framework is meant to be "extensible so you can easily 
>> add your own checks", it just seems like some sort of indication about 
>> which checks are run makes sense.  They are a great way for checking 
>> runtime conditions against production settings but not being able to 
>> confirm they actually ran somewhat negates the peace of mind that all is 
>> well.
>>
>> Thanks,
>> Gordon
>>
>>
>>
>> On Wednesday, June 10, 2020 at 11:59:39 AM UTC-4, Adam Johnson wrote:
>>>
>>> I am with Mariusz. Displaying the names of the check functions is a bit 
>>> against the intention of the checks framework. The check ID's are intended 
>>> to be enough information to reference the problem. This is different to 
>>> unit tests, where the function names are intended to carry information.
>>>
>>> Django doesn't really have a useful structure to its check function 
>>> names. For example a single check function, check_all_models() , is 
>>> responsible for all model level checks ( 
>>> https://github.com/django/django/blob/678c8dfee458cda77fce0d1c127f1939dc134584/django/core/checks/model_checks.py#L12
>>>  
>>> ). This has its own call tree through to Model.check() then each field's 
>>> Field.check(). Outputting a list entry like "check_all_models WARN" would 
>>> not give any actionable information.
>>>
>>> If you want to debug your own checks:
>>>
>>>1. Write tests for them. For example, use override_settings() in a 
>>>test case to set a setting to the value that would cause a warning, and 
>>>check that it returns the correct list of CheckMessage objects.
>>>2. You can inspect which check functions are registered with the 
>>>undocumented get_checks() function of the registry:
>>>
>>> In [1]: from django.core.checks.registry import registry
>>>
>>> In [2]: registry.get_checks()
>>> Out[2]:
>>> [>> **kwargs)>,
>>>  >> django.core.checks.urls.check_url_namespaces_unique(app_configs, **kwargs)>,
>>>  ...
>>>  >> **kwargs)>]
>>>
>>> If you're particularly unconfident that your checks.register() lines 
>>> don't always work, perhaps because they're conditional, you can also write 
>>> unit tests that run get_checks() and see your check functions are 
>>> registered.
>>>
>>> On Wed, 10 Jun 2020 at 15:00, Gordon  wrote:
>>>
 This is a discussion for https://code.djangoproject.com/ticket/31681 - 
 the feature would be useful to me and seems like something generally 
 useful 
 as well since it is implemented in the testing framework.  The closing 
 comment mentions that copying a documentation page into command output 
 isn't valuable.  That isn't what I meant to suggest so I will attempt to 
 explain more clearly below:

 It would be helpful if `checks` would output what checks it is 
 running.  Currently you don't actually know if a check runs or not.  
 Perhaps it isn't registered correctly?  Or it was excluded by mistake when 
 you expected it to run?  I like to keep this information around in build 
 history for quick inspection purposes as needed.


 For example, when running a check it would be nice to (optionally) see 
 something of the form:

- path.to.my.checkA ... 

Re: Add verbosity option to manage.py checks that outputs which checks are ran

[Adam Johnson]

This really feels like a "who watches the watchmen" problem.

Where does this problem stem from? Are you conditionally registering a
check function? It would be better in that case to move the conditionality
into the check body.


On Fri, 12 Jun 2020 at 15:33, Gordon  wrote:

> The underlying problem that I want to solve is a way to fail a CI job if
> checks for an app don't run.  The only approach I currently see that would
> accomplish that is by using a custom tag for every project that could fail
> with the invalid tag error - but this feels wrong.
>
> Would you be in favor of a flag for the check command that errors if
> checks aren't run from a particular namespace?
>
> Like `python manage.py check --require-checks foo` which would exit with a
> non-zero exit code if zero checks for `foo` are run?
>
>
>
> On Wednesday, June 10, 2020 at 6:40:16 PM UTC-4, Adam Johnson wrote:
>>
>> Perhaps there could be better on guidance in the documentation about
>> registering custom checks. I see nothing in the "writing your own checks"
>> guide really describes a recommended code structure. We'd accept a docs PR
>> there I'm sure.
>>
>> I normally have a checks submodule in my app and register the checks in
>> AppConfig.ready(). For example:
>> https://github.com/adamchainz/django-mysql/blob/2cf93c771f16011bc754a61e5aa95be4871f0363/src/django_mysql/apps.py#L17
>> . I don't like the decorator form since that's an import side effect and as
>> you've discovered it's not always easy to ensure your module gets imported.
>>
>> On Wed, 10 Jun 2020 at 21:16, Gordon  wrote:
>>
>>> Particular case that spurred this - reusable app had checks to make sure
>>> urls were reversible.  Checks weren't imported.  Checks command passes but
>>> doesn't run the apps checks.  There is no indication that the checks were
>>> not run so you have to assume they did and were successful.
>>>
>>> The app checks were tested for function.  Since this imports the checks
>>> they would show as registered using your `get_checks()` test, no?
>>> Seems kind of tricky...
>>>
>>> Since the checks framework is meant to be "extensible so you can easily
>>> add your own checks", it just seems like some sort of indication about
>>> which checks are run makes sense.  They are a great way for checking
>>> runtime conditions against production settings but not being able to
>>> confirm they actually ran somewhat negates the peace of mind that all is
>>> well.
>>>
>>> Thanks,
>>> Gordon
>>>
>>>
>>>
>>> On Wednesday, June 10, 2020 at 11:59:39 AM UTC-4, Adam Johnson wrote:

 I am with Mariusz. Displaying the names of the check functions is a bit
 against the intention of the checks framework. The check ID's are intended
 to be enough information to reference the problem. This is different to
 unit tests, where the function names are intended to carry information.

 Django doesn't really have a useful structure to its check function
 names. For example a single check function, check_all_models() , is
 responsible for all model level checks (
 https://github.com/django/django/blob/678c8dfee458cda77fce0d1c127f1939dc134584/django/core/checks/model_checks.py#L12
 ). This has its own call tree through to Model.check() then each field's
 Field.check(). Outputting a list entry like "check_all_models WARN" would
 not give any actionable information.

 If you want to debug your own checks:

1. Write tests for them. For example, use override_settings() in a
test case to set a setting to the value that would cause a warning, and
check that it returns the correct list of CheckMessage objects.
2. You can inspect which check functions are registered with the
undocumented get_checks() function of the registry:

 In [1]: from django.core.checks.registry import registry

 In [2]: registry.get_checks()
 Out[2]:
 [>>> **kwargs)>,
  >>> django.core.checks.urls.check_url_namespaces_unique(app_configs, 
 **kwargs)>,
  ...
  >>> django.contrib.auth.checks.check_user_model(app_configs=None, **kwargs)>]

 If you're particularly unconfident that your checks.register() lines
 don't always work, perhaps because they're conditional, you can also write
 unit tests that run get_checks() and see your check functions are
 registered.

 On Wed, 10 Jun 2020 at 15:00, Gordon  wrote:

> This is a discussion for https://code.djangoproject.com/ticket/31681 -
> the feature would be useful to me and seems like something generally 
> useful
> as well since it is implemented in the testing framework.  The closing
> comment mentions that copying a documentation page into command output
> isn't valuable.  That isn't what I meant to suggest so I will attempt to
> explain more clearly below:
>
> It would be helpful if `checks` would output what checks it is
> running.  Currently you don't actuall

Re: Proposal - Warn user when creating or applying a delete migration?

[Denis Urman]

It's easy enough to read the output before you migrate. It's pretty darn 
hard to accidentally delete an entire Model. It'd have to be a new or 
obscure Model with no references in your Views, no ForeignKeys to it, etc 
etc. Deleting a Field accidentally on the other hand is pretty easy to do. 
I do feel that migrations are a major sore point in Django. I don't believe 
that *"*Make sure to read the output to see what makemigrations* thinks you 
have changed - it’s not perfect, and for complex changes it might not be 
detecting what you expect." * is good enough for us. A flag/option for this 
wouldn't hurt.

-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/6262515b-ca57-4708-8240-fb618115cc54o%40googlegroups.com.