#22568 closed Bug (fixed)
FK to proxy model breaks migrate
Reported by: | leftmoose | Owned by: | Andrew Godwin |
---|---|---|---|
Component: | Migrations | Version: | 1.7-beta-2 |
Severity: | Release blocker | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | yes | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
Situation similar to #22527
but for new 1.7 migrations
installed_apps = ( ... app1, app2, app3 ) db: postgres (to enfoce constraints) app1.models: from django.db import models class Model1(models.Model): foreign2 = models.ForeignKey('app2.Model2') app2.models: from app3.models import Model3 class Model2(Model3): class Meta: proxy = True app3.models: from django.db import models # Create your models here. class Model3(models.Model): pass
$ manage.py makemigrations app3 $ manage.py makemigrations app1 $ ./manage.py test Creating test database for alias 'default'... Traceback (most recent call last): File "./manage.py", line 10, in <module> execute_from_command_line(sys.argv) File "/Users/david/dev/django_source/django/core/management/__init__.py", line 427, in execute_from_command_line utility.execute() File "/Users/david/dev/django_source/django/core/management/__init__.py", line 419, in execute self.fetch_command(subcommand).run_from_argv(self.argv) File "/Users/david/dev/django_source/django/core/management/commands/test.py", line 50, in run_from_argv super(Command, self).run_from_argv(argv) File "/Users/david/dev/django_source/django/core/management/base.py", line 288, in run_from_argv self.execute(*args, **options.__dict__) File "/Users/david/dev/django_source/django/core/management/commands/test.py", line 71, in execute super(Command, self).execute(*args, **options) File "/Users/david/dev/django_source/django/core/management/base.py", line 337, in execute output = self.handle(*args, **options) File "/Users/david/dev/django_source/django/core/management/commands/test.py", line 88, in handle failures = test_runner.run_tests(test_labels) File "/Users/david/dev/django_source/django/test/runner.py", line 147, in run_tests old_config = self.setup_databases() File "/Users/david/dev/django_source/django/test/runner.py", line 109, in setup_databases return setup_databases(self.verbosity, self.interactive, **kwargs) File "/Users/david/dev/django_source/django/test/runner.py", line 297, in setup_databases verbosity, autoclobber=not interactive) File "/Users/david/dev/django_source/django/db/backends/creation.py", line 368, in create_test_db test_database=True) File "/Users/david/dev/django_source/django/core/management/__init__.py", line 167, in call_command return klass.execute(*args, **defaults) File "/Users/david/dev/django_source/django/core/management/base.py", line 337, in execute output = self.handle(*args, **options) File "/Users/david/dev/django_source/django/core/management/commands/migrate.py", line 145, in handle executor.migrate(targets, plan, fake=options.get("fake", False)) File "/Users/david/dev/django_source/django/db/migrations/executor.py", line 60, in migrate self.apply_migration(migration, fake=fake) File "/Users/david/dev/django_source/django/db/migrations/executor.py", line 88, in apply_migration if self.detect_soft_applied(migration): File "/Users/david/dev/django_source/django/db/migrations/executor.py", line 132, in detect_soft_applied apps = project_state.render() File "/Users/david/dev/django_source/django/db/migrations/state.py", line 52, in render raise InvalidBasesError("Cannot resolve bases for %r" % new_unrendered_models) django.db.migrations.state.InvalidBasesError: Cannot resolve bases for [<django.db.migrations.state.ModelState object at 0x10b2a8dd0>]
Change History (13)
comment:1 by , 11 years ago
comment:2 by , 11 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:3 by , 11 years ago
what's the criteria for release blockers? this breaks foreign keys to proxy models...
comment:4 by , 11 years ago
A possible solution is to deconstruct foreign keys to point to the concrete model instead. Proxy models are used to override methods on the model. Migrations do not include custom methods. So having the foreign key directly to the concrete base model should be good enough.
The problem with this approach is that when descontructing the ForeignKey it is possible that only a string reference is known (that is, the self.rel.to is something like 'auth.User'). From the string representation it is impossible to know if the referenced model is a proxy model. When resurrecting the string from the migration file there is also no knowledge if the model is a proxy model.
I am no expert on migrations, but my understanding is that there is no fundamental reason why the proxy models couldn't be included in a way that they are only loaded to the app-cache. This might be hard and/or ugly to do, that might be a reason why it wasn't done. More likely the reason was that proxy models do not need any database changes, so there is no need to have them in migrations. But this ticket seems to contradict that assumption.
comment:5 by , 11 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:6 by , 11 years ago
Severity: | Normal → Release blocker |
---|
Just talked this through with davids on IRC; we've concluded that the probable best approach is some new operations, CreateProxyModel and DeleteProxyModel, and to have the autogenerator create those as it sees proxies come and go (there's no need for rename, as there's no data to preserve, we can just delete/create).
This is mainly because migrations can't deal with abstract inheritance well generally, and this is a cleaner way of inserting these into the ORM.
Also bumping to release blocker.
comment:7 by , 10 years ago
Has patch: | set |
---|---|
Needs tests: | set |
Patch needs improvement: | set |
https://github.com/django/django/pull/2645/
needs review + more tests
comment:8 by , 10 years ago
I'd prefer a proxy=True
attr and a couple of if
statements over CreateProxyModel
+ DeleteProxyModel
and RenameProxyModel
. There is some code duplication and with the logic in two different places it's a lot easier to forget about proxy models.
comment:9 by , 10 years ago
seems worth a try. i also wonder if relationships to managed = False
models suffer the same problem and need a similar solution. so maybe this becomes orm_only = True
or similar
comment:10 by , 10 years ago
I'm going to claim ownership of this as I intend to look at this in the next week.
loic84: I'm going to investigate if we can get proxy=True working, but I remember that I thought there'd be a problem with that (but I can't remember what my supposed problem was). Given that #22470 means we need a general "change model options" operation, I know we'd need to make sure changing proxy=True to False and vice-versa was detected as an add and a delete, not as a change, as otherwise SchemaEditor will get itself in a twist trying to alter it.
comment:11 by , 10 years ago
Owner: | changed from | to
---|
comment:12 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
looking into this one possible cause might be that proxy models are ignored by
makemigrations
. (explicitly skipped inMigrationAutodetector._detect_changes
). that means the derived stateloader.graph.project_state()
doesn't know about the proxy model.what's the reason for skipping proxy models (as opposed to creating migrations for them with no fields)?