[Django] #22837: Migrations detect unnessecary(?) changes
Django
-------------------------------+------------------------
Reporter: valberg | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.7-beta-2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------
I don't know if this is intended behavior, but having a simple model as:
{{{
class Foo(models.Model):
bar = models.SlugField()
}}}
Simple changes, that do have no impact on the database representation,
result in new migrations. For instance:
{{{
class Foo(models.Model):
bar = models.SlugField(editable=False)
}}}
results in:
{{{
# -*- coding: utf-8 -*-
from __future__ import unicode_literals
from django.db import models, migrations
class Migration(migrations.Migration):
dependencies = [
('testapp', '0001_initial'),
]
operations = [
migrations.AlterField(
model_name='foo',
name='bar',
field=models.SlugField(editable=False),
),
]
}}}
And further:
{{{
class Foo(models.Model):
bar = models.SlugField(
editable=False,
choices=[
('baz', 'Baz'),
('test', 'Test'),
]
)
}}}
Results in:
{{{
# -*- coding: utf-8 -*-
from __future__ import unicode_literals
from django.db import models, migrations
class Migration(migrations.Migration):
dependencies = [
('testapp', '0002_auto_20140614_2237'),
]
operations = [
migrations.AlterField(
model_name='foo',
name='bar',
field=models.SlugField(editable=False, choices=[('baz',
'Baz'), ('test', 'Test')]),
),
]
}}}
It is as if the detector does "too much" detecting. But again there might
well be a good reason for this. Just thought it would be a good thing to
address :)
--
Ticket URL: <https://code.djangoproject.com/ticket/22837>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: valberg | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.7-beta-2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by bendavis78):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
From what I understand, the migrations system doesn't discriminate between
field attributes, so if any attribute changes (even ones like help text),
the auto-detector will treat it like any other change. There's currently
no elegant way to predict how field attributes might affect the migration
states. See #21498 for more discussion on the subject.
--
Ticket URL: <https://code.djangoproject.com/ticket/22837#comment:1>
Django
Reporter: valberg | Owner: nobody
Type: Uncategorized | Status: closed
Component: Migrations | Version: 1.7-beta-2
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by mjtamlyn):
* status: new => closed
* resolution: => invalid
Comment:
This is by design. There are several reasons, not least of which for me
that datamigrations at points in history need to have a full accurate
representation of the models, including all their options not just those
which affect the database.
[Bendavis: if, as I read from your comment, you believed this was not a
valid bug, you are welcome to close the ticket yourself]
--
Ticket URL: <https://code.djangoproject.com/ticket/22837#comment:2>
Django
Reporter: valberg | Owner: nobody
Type: Uncategorized | Status: closed
Component: Migrations | Version: 1.7-beta-2
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by pykler):
To me this is future proofing since no DBMS (that I know of) has such a
constraint on char fields (which is what is used here).
Tell me what the correct behaviour is in the following scenario (according
to the ticket close comment, this is a scenario that is designed for).
{{{
0001 migration choices are ('a', 'apple'), ('b', 'banana')
0002 data insert into table choice 'banana'
0003 migration model has only one choice now ('a', 'apple')
}}}
From my tests, django doesn't (and 100% defenitely shouldn't) be deleting
data or replacing data in every record of my database table on migration
0003. So the reality is, this point in time validation check is really
only for checking if the choice is valid during the data migration which
isn't really useful.
The problem with this design is some apps (third party) will have a
dynamic set of choices depending on your settings, so they cannot reliably
deliver a set of migrations causing the user of the app to have to create
migrations on behalf of the third party app.
Surely there is a better design out there to solve the scenario where a
datamigration is somehow validated and no grief is caused to django users.
PS. I believe django migrations are very elegant even with this little
design snafu https://twitter.com/pykler/status/546868601492627456
--
Ticket URL: <https://code.djangoproject.com/ticket/22837#comment:3>
Django
Reporter: valberg | Owner: nobody
Type: Uncategorized | Status: closed
Component: Migrations | Version: 1.7-beta-2
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MarkusH):
Replying to [comment:3 pykler]:
> Tell me what the correct behaviour is in the following scenario
(according to the ticket close comment, this is a scenario that is
designed for).
>
> {{{
> 0001 migration choices are ('a', 'apple'), ('b', 'banana')
> 0002 data insert into table choice 'banana'
> 0003 migration model has only one choice now ('a', 'apple')
> }}}
>
> From my tests, django doesn't (and 100% defenitely shouldn't) be
deleting data or replacing data in every record of my database table on
migration 0003. So the reality is, this point in time validation check is
really only for checking if the choice is valid during the data migration
which isn't really useful.
Django will **not** go through your database table and remove all rows
with 'banana'. But you could add another migration that selects all rows
that don't have valid data and work with them on your own. And you can
only do that if you have `choices` in your field definition.
> The problem with this design is some apps (third party) will have a
dynamic set of choices depending on your settings, so they cannot reliably
deliver a set of migrations causing the user of the app to have to create
migrations on behalf of the third party app.
They can, by passing a callable to `choices`. See #13181
--
Ticket URL: <https://code.djangoproject.com/ticket/22837#comment:4>
Django
Reporter: valberg | Owner: nobody
Type: Uncategorized | Status: closed
Component: Migrations | Version: 1.7-beta-2
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by guettli):
Unfortunately Django 1.7 does not allow choices to be a callable. This was
introduced in Django 1.8.
That's sad.
--
Ticket URL: <https://code.djangoproject.com/ticket/22837#comment:5>
Django
Reporter: valberg | Owner: nobody
Type: Uncategorized | Status: closed
Component: Migrations | Version: 1.7-beta-2
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aidanlister):
I hate this! There's several modules I've used where changing something in
settings starts generating migrations in impossible places (inside
virtualenvs, on heroku dynos, etc). There needs to be a better solution to
this ... I really can't see the value in having choices exposed to the
migrations if it's not wanted (e.g. a callable is used).
--
Ticket URL: <https://code.djangoproject.com/ticket/22837#comment:6>
Django
Reporter: valberg | Owner: nobody
Type: Uncategorized | Status: closed
Component: Migrations | Version: 1.7-beta-2
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Josh Smeaton):
I've raised the lack of modelfield.choices on the mailing list with the
view to reopen this ticket: https://groups.google.com/forum/#!topic
/django-developers/vKsxJBafY9g
--
Ticket URL: <https://code.djangoproject.com/ticket/22837#comment:7>
Django
Reporter: valberg | Owner: nobody
Type: Uncategorized | Status: closed
Component: Migrations | Version: 1.7-beta-2
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Josh Smeaton):
* cc: josh.smeaton@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/22837#comment:8>
Django
Reporter: valberg | Owner: nobody
Type: Uncategorized | Status: closed
Component: Migrations | Version: 1.7-beta-2
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
If there's consensus about that change, I would open a new ticket rather
than reopen this one since the description and comments here have lots of
unrelated discussion.
--
Ticket URL: <https://code.djangoproject.com/ticket/22837#comment:9>
Django
Reporter: valberg | Owner: nobody
Type: Uncategorized | Status: closed
Component: Migrations | Version: 1.7-beta-2
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Sergey Fedoseev):
Replying to [comment:9 Tim Graham]:
> If there's consensus about that change, I would open a new ticket rather
than reopen this one since the description and comments here have lots of
unrelated discussion.
I think we already have open ticket for this: #24561.
--
Ticket URL: <https://code.djangoproject.com/ticket/22837#comment:10>