Opened 9 years ago
Last modified 2 years ago
#23521 assigned Bug
removal of concrete Model from bases doesn't remove it from ModelState bases
Reported by: | Sergey Fedoseev | Owned by: | Vadim Fabrichnov |
---|---|---|---|
Component: | Migrations | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | info+coding@…, bugs@…, ericmills2@…, Charlie Denton, Ian Foote, Daniel Rios, Sardorbek Imomaliev | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | yes | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
Steps to reproduce:
- add
test
app withmodels.py
:class Thing(models.Model): pass class SuperThing(Thing): pass
- do
manage.py makemigrations test
- change
models.py
(removeThing
):class SuperThing(models.Model): thing_ptr = models.AutoField(primary_key=True)
- do
manage.py makemigrations test
- do it one more time
manage.py makemigrations test
last command results in
django.db.migrations.state.InvalidBasesError: Cannot resolve bases for [<ModelState: 'test.SuperThing'>] This can happen if you are inheriting models from an app with migrations (e.g. contrib.auth) in an app with no migrations; see https://docs.djangoproject.com/en/1.7/topics/migrations/#dependencies for more
Change History (29)
comment:1 Changed 9 years ago by
Component: | Uncategorized → Migrations |
---|---|
Type: | Uncategorized → Bug |
comment:2 follow-up: 3 Changed 9 years ago by
comment:3 Changed 9 years ago by
Replying to bmispelon:
Hi,
Probably this is not obvious, but removal of Thing
model from models.py
, not only from SuperThing
bases was implied.
As a side note, it's not possible to create an app called
test
because you get this error when doingmanage.py startapp test
I have never used this for creating apps =)
comment:4 Changed 9 years ago by
Triage Stage: | Unreviewed → Accepted |
---|
Thanks, that's the bit I was missing.
comment:5 Changed 9 years ago by
Cc: | info+coding@… added |
---|---|
Description: | modified (diff) |
comment:6 follow-up: 7 Changed 9 years ago by
Are you working on a patch sir-sigurd? If not, I'd write one.
comment:7 Changed 9 years ago by
Replying to Markush2010:
Are you working on a patch sir-sigurd? If not, I'd write one.
Yes, I'm working on it.
comment:8 Changed 9 years ago by
There's an easier way to reproduce what seems to be the exact same error:
- Create a
BaseModel
and aSubModel
that inherits from it - Run
makemigrations
andmigrate
to add both models to database - Delete both models together
- Run
makemigrations
andmigrate
to delete both models from the database
The same error should now appear.
comment:9 Changed 9 years ago by
I invite you all to look at my marked-as-duplicate bug #23818. To me it seems there is simply no way for django's migration operations to express a change of bases
. parent_link
s are created & removed, but the built-up in-memory representation of the models never understands the new parentage, meaning certain operations (e.g. in a RunPython operation) will simply not work right when referring to inherited fields. This goes beyond makemigrations
simply not detecting a change.
comment:10 Changed 9 years ago by
Cc: | bugs@… added |
---|
comment:13 Changed 7 years ago by
Cc: | ericmills2@… added |
---|
This bug has burned me pretty bad. I was working through steps to switch from concrete inheritance to abstract inheritance. Is someone going to revisit this?
comment:15 Changed 7 years ago by
Having the same issue.
I had to manually alter my initial migration and remove the offending base class for my app to work.
comment:16 Changed 6 years ago by
I had the need as described in #23818 but instead of squashing migrations i've created a migrations operator to update the bases on migrations models state. This works fine for me on django 1.8. I still haven't tested if django master behaves differently. Do you think this is a valuable addition to django?
class AlterBaseOperation(Operation): reduce_to_sql = False reversible = True def __init__(self, model_name, bases, prev_bases): self.model_name = model_name self.bases = bases self.prev_bases = prev_bases def state_forwards(self, app_label, state): state.models[app_label, self.model_name].bases = self.bases state.reload_model(app_label, self.model_name) def state_backwards(self, app_label, state): state.models[app_label, self.model_name].bases = self.prev_bases state.reload_model(app_label, self.model_name) def database_forwards(self, app_label, schema_editor, from_state, to_state): pass def database_backwards(self, app_label, schema_editor, from_state, to_state): pass def describe(self): return "Update %s bases to %s" % (self.model_name, self.bases)
comment:19 Changed 4 years ago by
Cc: | Charlie Denton added |
---|
comment:20 Changed 4 years ago by
Cc: | Ian Foote added |
---|
comment:21 Changed 4 years ago by
Owner: | set to Ian Foote |
---|---|
Status: | new → assigned |
comment:23 Changed 4 years ago by
Patch needs improvement: | set |
---|---|
Version: | 1.7 → master |
comment:24 Changed 4 years ago by
Another use case of migrations.AlterModelBases()
is described in #30513.
comment:25 Changed 4 years ago by
Cc: | Daniel Rios added |
---|
comment:26 Changed 4 years ago by
There is one more case that needs to be addressed related to bases in migrations.
Step 1:
Consider this model:
class Good(GoodServiceBase): title = models.CharField(max_length=16)
this creates migration with the following operation:
migrations.CreateModel( name='Good', fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), ('title', models.CharField(max_length=16)), ], ),
Step 2:
Then we add a new model and change existing model to inherit from the new model.
class GoodServiceBase(models.Model): active = models.BooleanField(default=True) class Good(GoodServiceBase): title = models.CharField(max_length=16)
This generates migration like this:
migrations.CreateModel( name='GoodServiceBase', fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), ('active', models.BooleanField(default=True)), ], ), migrations.RemoveField( model_name='good', name='id', ), migrations.AddField( model_name='good', name='goodservicebase_ptr', field=models.OneToOneField(auto_created=True, default=1, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='goods.GoodServiceBase'), preserve_default=False, ),
If we have no data in database, this will pass.
Step 3 with crash:
Try to access field active
in datamigration and migration crashes. Operation in migration to reproduce this:
def migrate_my_data(apps, schema_editor): for good in apps.get_model("goods", "Good").objects.all(): print(good.active) migrations.RunPython(migrate_my_data, reverse_code=lambda x, y: None)
This crashes with the error:
<...> File "<project home>/goods/migrations/0003_auto_20191025_1157.py", line 8, in migrate_my_data print(good.active) AttributeError: 'Good' object has no attribute 'active'
As far as I understand this happens because bases
is not altered in migration and fake models do not inherit fields from their true parent models. Thus, currently if we change inheritance chain of existing models, we lose the ability to use those models in data migrations.
Tested in version 2.2.6.
comment:29 Changed 3 years ago by
Owner: | Ian Foote deleted |
---|---|
Status: | assigned → new |
Deassigning because I don't think I'm likely to get to this again in the near future and I'd be happy for someone else to take my work and finish the job.
comment:30 Changed 3 years ago by
Cc: | Sardorbek Imomaliev added |
---|
comment:31 Changed 3 years ago by
Owner: | set to Vadim Fabrichnov |
---|---|
Status: | new → assigned |
comment:32 Changed 2 years ago by
Patch needs improvement: | unset |
---|
comment:33 Changed 2 years ago by
Needs tests: | set |
---|
Hi,
I can't reproduce the issue you're describing.
On master, the second
makemigrations
doesn't detect any changes.On
stable/1.7.x
, it does create a migration but running it a third time works (and detects no change as expected).As a side note, it's not possible to create an app called
test
because you get this error when doingmanage.py startapp test
:Can you provide us with some details on how you're trigerring the issue?
Thanks.