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.

[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.

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

--

Adam

[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

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAMyDDM0ROmXKTaZ7fuw7cvXLLRk8AbrWw1EhcQMssOofpYGKGA%40mail.gmail.com.

[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

[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.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/69C34E37-ADE5-4B56-8439-5F8CC962017C%40tomforb.es.

--

Adam

[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

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAMyDDM2E8-h2tqpSDzHKySA8PUPGy2y1pWdt5rxX6aY9U-WXRg%40mail.gmail.com.

[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 :-)

[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.

[Carlton Gibson]

Looking at the proposed ticket here #31700, I'm Meh at best:

* - + ~

* Green, yellow, red

I'm not sure I want to look at either of those. Visual noise for X benefit, where I think X is likely very small.

Not sure it's worth the complication.

I'll close as needsinfo. If there's a lot of +1s here then we can reopen and accept.

Thanks.

C.

[sourav tiwari]

I m in search of job Can any one help me to find out a job

--
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 view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/91028857-8ef6-42cc-991c-3c092370341co%40googlegroups.com.

[Tobias McNulty]

I'm +1 to doing *something.* Absent a louder reminder, I think it's unrealistic to expect everyone to read the output of makemigrations all the time.

As others have said, I'm not sure `manage.py migrate` is the right time. I think it's too late. The code may have already been committed, who knows what machine the migrations are being run on, etc.

During makemigrations, on the other hand, I don't see anything wrong with formatted text or +/-, but I might go a step further. We already prompt for things like renames and merges. Why not forcibly gain user acceptance before creating a migration that deletes something? We could repeat the reminder to double check it or allow the user to cancel the migration altogether, for example:

"You are creating a migration that DELETES one or more columns or database tables. As with any migration, if you continue, you should inspect the created migration BEFORE running `manage.py migrate` to ensure it does what you expect. Press Y to continue or N to cancel." (needs copyediting of course)

It would be easy enough to bypass with --no-input if someone doesn't want the interruption.

Cheers,

Tobias McNulty
Chief Executive Officer

tob...@caktusgroup.com
www.caktusgroup.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-develop...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/91028857-8ef6-42cc-991c-3c092370341co%40googlegroups.com.

[René Fleschenberg]

Hi,

> During makemigrations, on the other hand, I don't see anything wrong
> with formatted text or +/-, but I might go a step further.

This sounds like a good idea to me. It would break cases where
makemigrations is run non-interactively, which might be a good thing.
It's a common mistake to run makemigrations && migrate on deployment
instead of committing the migrations to version control.

Regards,
René