Hey community!
Based on this ticket, I added a feature that highlights safe/possibly destructive/destructive migration operations when you do makemigartions. I think it’s a good idea to ask the community’s opinion about such a feature. Do you think such a feature it’s good to exist or add noise (even if it is configurable)?
This sounds interesting.
Would safety only be defined in relation to data integrity? For example would there be a color for a not nullable column being added to a table because it takes a table lock on migrate?
So far, adding a column to a table is considered a safe operation with a + sign and green color.
Please consider that there can be no color warning (if you think it’s too much), and just +/-/~ signs to indicate the effect of an operation on the data of a table.
To jump on this: if Django was able to bubble up what kind of locks would be taken to run a set of migrations that would be somewhat amazing. I have experienced way too many “oh actually this locks the table” moments right as a migration starts in production in my lifetime
@AMK9978 I like the idea. A little bit of colour and -/+ signs would make the output more readable and could highlight accidental removals.
I am not entirely sold on the safe/possibly destructive/destructive categorization, at least the names. Even adding a table may be “unsafe” and crash, if your database user doesn’t have permissions to create tables.
I would prefer to stick with only “add“ and “remove” categories. The +/- could then be read more like a diff.
I would always count a RenameField as destructive, since it removes the old name, preventing any currently running deployment from using the name it relied on. Also, it’s probably best to always count RunSQL as destructive, because a yellow “possibly destructive“ highlight could be easily ignored.
I’m afraid that is likely impossible in general. It will depend on the database backend, version, storage engine, configuration, and many other features outside of Django’s control. To grab that information it’s probably best to actually run the migration against a real database server, such as on a dev or staging environment, and inspect the locks held.
@adamchainz
I liked the idea of showing with +/- like Git diff, but as you mentioned, there are operations that are a combination of add and remove (updates) or none like RunSQL. I think we’d better put them in a category when the report is shown to the developer.
Maybe we need to find consensus on the format of the makemigrations output. For example, it can be composed of four sections, added, removed, updated, and alert(!) ones. Or, we can decompose RunSQL and update operations to added and removed ones (like Git), but I think this is too much and noisy, or any other idea.
Thoughts?
I’d rather keep it simple. So many things are specific to the database backend and configuration.
For example, many operations on SQLite need to create a second table, copy data into it, swap the names, and drop the old one. That’s always potentially long-running or destructive.
I don’t think this is feasible. It would require parsing arbitrary SQL which can include all kinds of syntax, even kinds we can’t predict on third party database backends.
@adamchainz
So you suggest that it’s just enough to cover only add and remove operations? I think when there are signs and maybe colors for such, there should be some kind of notice for the rest of operations. For example, the report should be ordered: added, removed, and the rest. Plus, at the end of this format, there can be a kind of notice for RawSQL as it might lead to unwanted consequences.
Any suggestion for such a format will be appreciated! I’d rather to begin simply too.
I suggest we only have two groups as-is. The main purpose would be to spot migrations that are definitely not removing anything. Adding a third ambiguous category seems like overkill and a potential source of confusion.
Also I don’t think we should reorder operations in the output, but keep them in the order they will be applied. Reordering could be misleading.
@adamchainz
Updated. I took two categories: removal and non-removal (only adding). Thoughts?
I would appreciate it if other Django developers, especially, the board members leave their comments as it’s a kinda UI/UX feature.
Thanks for the ping @adamchainz
I’m not against the usage of symbols to denote addition of removal but I’m not sure the usage of colors such as green to denote addition is a good thing because of how it can be associated to safeness.
The example Adam give about RenameField is also true for non-nullable field addition as pointed out by @CodenameTim. If the newly introduced db_default option is not used when a new field is added to a table no insert can succeed with the old deployed code but the new code can’t be deployed until the migration lands. This is just an example of how additions can also be unsafe and why I’m uneasy about using green here.
I understand how its meant to be a diff analogy but we need to consider two kind of safetyness that must be taken into account when making schema changes
Django’s migration framework doesn’t provide any tooling for 1. and very little for 2. on some backends for the operations it automatically generates. With this in mind I’m a bit hesitant about implicit guarantees that some operations are safe.
To summarize, +1 on using symbols (+, -, ~, p, ?, s, etc) to allow for easier visual parsing of makemigrations’s output and -0 on using green for additive changes.
I think that’s totally fair. Good example with adding a new non-nullable non-db-defaulted field.
I guess you’re proposing:
+ - addition
- - removal
~ - rename
p - python
? - SeparateDatabaseAndState or custom operation without the attr defined
s - SQL
I guess you’re proposing
Yes, something along these lines. The exact symbols are definitely up for discussions though, it just seemed like good candidates from a quick scan through django.db.models.operations
To get a sense of the output in such a way:
The idea behind categorizing (and colonizing) the highlight was to alert developers when they use the makemigrations command (and usually migrate right after that). Does such output give enough warning to developers? If we colorize it, there may be three different colors after makemigration command in the output (addition, removal, rename).
Btw, isn’t alter a better name for the rename category?
Like always, any idea on this would be appreciated!
I like the idea!
On the colour pallette, Michael Nicholson did a good lightning talk that we should be concious of the colour blind with red and green (https://youtu.be/acQdzpt68Fk?si=hOD1_S9RDyy8YGxM) so maybe best to avoid green with red. Maybe red with amber or red with a light blue might work? (There must be ways to test it)
Would also remove the messaging of green=good/safe
But the additional symbols maybe already acheive the goal of being colour blind friendly 
According accessibility, additional symbols should be enough to me.
For the colors, I’m not 100% sure of the answer, I think it really depends on the user’s terminal configuration colors. I’ll defer on the rest of the accessibility team on this part.
Thank you, everyone, for your engagement. Apparently, the prevailing idea is to use symbols by default, and I’m eager to advance this issue in Django. Is there any other idea on categorizing or any objection?
If you’ve addressed Adam’s comments in the PR (I didn’t check), the next thing to do would be to uncheck “Patch needs improvement” on Trac so it gets back in the review queue.
