[Django] #34568: makemigrations --update changes the name of custom migration name
Django
-------------------------------------+-------------------------------------
Reporter: David | Owner: nobody
Sanders |
Type: | Status: new
Uncategorized |
Component: Database | Version: 4.2
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
This may be intentional behaviour but the docs don't mention this so
creating a ticket to update docs or correct behaviour:
If you create a migration with a custom name:
{{{
$ ./manage.py makemigrations --name foo
Migrations for 'update_rename':
update_rename/migrations/0001_foo.py
- Create model Foo
}}}
then running `--update` will change the name "foo" to the autogenerated
one based on the operations:
{{{
$ ./manage.py makemigrations --update
Migrations for 'update_rename':
update_rename/migrations/0001_initial.py
- Create model Foo
Deleted update_rename/migrations/0001_foo.py
}}}
My opinion is that it shouldn't as it violates the principle of least
astonishment ''even though the `--name` argument wasn't supplied''.
--
Ticket URL: <https://code.djangoproject.com/ticket/34568>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Uncategorized | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by David Sanders):
This is my first time using `--update` and here are a few other
observations which could indicate that it requires broader discussion:
- It doesn't utilise the `--name` argument so it's impossible to
customise the name during `--update`
- It'd be nice to provide `--no-optimize` option to `--update`, here's my
use-case: 3-step non-null field addition. After doing nullable step 1,
elidable data migration step 2, I want to merge the non-null update into
the migration but `--update` optimizes this into a single step.
Perhaps `--update` requires a rethink?
--
Ticket URL: <https://code.djangoproject.com/ticket/34568#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: David Sanders | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> This may be intentional behaviour but the docs don't mention this so
> creating a ticket to update docs or correct behaviour:
>
> If you create a migration with a custom name:
>
> {{{
> $ ./manage.py makemigrations --name foo
> Migrations for 'update_rename':
> update_rename/migrations/0001_foo.py
> - Create model Foo
> }}}
>
> then running `--update` will change the name "foo" to the autogenerated
> one based on the operations:
>
> {{{
> $ ./manage.py makemigrations --update
> Migrations for 'update_rename':
> update_rename/migrations/0001_initial.py
> - Create model Foo
> Deleted update_rename/migrations/0001_foo.py
> }}}
>
> My opinion is that it shouldn't as it violates the principle of least
> astonishment ''even though the `--name` argument wasn't supplied''.
New description:
This may be intentional behaviour but the docs don't mention this so
creating a ticket to update docs or correct behaviour:
If you create a migration with a custom name:
{{{
$ ./manage.py makemigrations --name foo
Migrations for 'update_rename':
update_rename/migrations/0001_foo.py
- Create model Foo
}}}
then running `--update` will change the name "foo" to the autogenerated
one based on the operations:
{{{
$ ./manage.py makemigrations --update
Migrations for 'update_rename':
update_rename/migrations/0001_initial.py
- Create model Foo
Deleted update_rename/migrations/0001_foo.py
}}}
My opinion is that it shouldn't as it violates the principle of least
astonishment ''even though the `--name` argument wasn't supplied''.
EDIT:
This is my first time using --update and here are a few other observations
which could indicate that it requires broader discussion:
- It doesn't utilise the --name argument so it's impossible to customise
the name during --update
- It'd be nice to provide --no-optimize option to --update, here's my
use-case: 3-step non-null field addition. After doing nullable step 1,
elidable data migration step 2, I want to merge the step 3 non-null update
into the migration but --update optimizes this into a single step.
Perhaps --update requires a rethink?
--
--
Ticket URL: <https://code.djangoproject.com/ticket/34568#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: David Sanders | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: David Wobrock (added)
Comment:
I think you're expecting too much from `--update` was intended to be an
option for updating recently developed changes and don't do anything
fancier. Please check the discussion in
[https://github.com/django/django/pull/15669 PR].
> My opinion is that it shouldn't as it violates the principle of least
astonishment even though the --name argument wasn't supplied.
We could document this behavior, but IMO it shouldn't be changed.
> It'd be nice to provide --no-optimize option to --update, here's my use-
case: 3-step non-null field addition. After doing nullable step 1,
elidable data migration step 2, I want to merge the step 3 non-null update
into the migration but --update optimizes this into a single step.
It's probably worth adding 🤔
--
Ticket URL: <https://code.djangoproject.com/ticket/34568#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: David Sanders | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by David Sanders):
@felix read through that PR, I'm assuming your point about ending up with
a name that doesn't match the operations is referring to not being able to
tell the difference between a custom name and an auto gen'd name. Here's
my response:
- It's a fair point. Trying to determine custom names is too complex and
shouldn't be done.
- In my anecdotal experience we (myself and colleagues) rarely use the
auto gen'd name. An option to `--keep-name` sounds nice to me with the
default to *not keep*.
- If not, then at the very least supplying `--name` should raise an
exception or honour the name. It definitely shouldn't do *nothing*. My
preference is to honour `--name` 😊
Can we get Simon's take as well since he was part of the thread?
I still think something should change because it's not even fancy… I'd say
that turning off optimisations is harder than this.
--
Ticket URL: <https://code.djangoproject.com/ticket/34568#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: David Sanders | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by David Sanders):
Also just now thought of another option:
- An option to print migrations to stdout `--stdout` (or whatever) then I
can redirect the content to my latest migration and edit as necessary 😅
--
Ticket URL: <https://code.djangoproject.com/ticket/34568#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: David Sanders | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Simon Charette (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/34568#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: David Sanders | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Natalia Bidart):
After reading the referenced PR and the comments in this ticket, I agree
with David that ignoring `--name` feels like a bug, and I think it's worth
fixing. I believe the fix would be as simple as (plus tests):
{{{
#!diff
--- a/django/core/management/commands/makemigrations.py
+++ b/django/core/management/commands/makemigrations.py
@@ -316,7 +316,7 @@ class Command(BaseCommand):
)
# Update name.
previous_migration_path =
MigrationWriter(leaf_migration).path
- suggested_name = (
+ suggested_name = self.migration_name or (
leaf_migration.name[:4] + "_" +
leaf_migration.suggest_name()
)
if leaf_migration.name == suggested_name:
}}}
Regarding optimization, I would have taken a different approach in the
original implementation: do not optimize the updated migration, leave that
decision to the caller, they can later call `optimizemigration`. Is it too
late for considering that approach?
Lastly, I would not go the route of allowing to keep the current name, the
caller can achieve that by passing `--name` which is already a documented
and valid option for the command.
--
Ticket URL: <https://code.djangoproject.com/ticket/34568#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: David Sanders | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
I also think that `--name` should not be ignored when `--update` is used
but updating the name of the updated migration to follow the usual
operation fragment stitching otherwise seems like a expected default.
Whether or not we we should add an option not to perform optimization or
disable operation fragment usage for migration name entirely seems like
distinct feature requests.
--
Ticket URL: <https://code.djangoproject.com/ticket/34568#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: David Sanders | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by David Sanders):
Replying to [comment:8 Simon Charette]:
> Whether or not we we should add an option not to perform optimization or
disable operation fragment usage for migration name entirely seems like
distinct feature requests.
Yep, though before I create a ticket for that, I might just create a PoC
to see whether it helps with the use case I described above.
--
Ticket URL: <https://code.djangoproject.com/ticket/34568#comment:9>