Opened 19 months ago

Closed 18 months ago

Last modified 18 months ago

#22568 closed Bug (fixed)

FK to proxy model breaks migrate

Reported by: leftmoose Owned by: andrewgodwin
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


Situation similar to #22527

but for new 1.7 migrations

installed_apps = (
    app1, app2, app3

db: postgres (to enfoce constraints)

from django.db import models

class Model1(models.Model):
    foreign2 = models.ForeignKey('app2.Model2')

from app3.models import Model3

class Model2(Model3):
    class Meta:
        proxy = True

from django.db import models

# Create your models here.
class Model3(models.Model):
$ makemigrations app3
$ makemigrations app1

$ ./ test
Creating test database for alias 'default'...
Traceback (most recent call last):
  File "./", line 10, in <module>
  File "/Users/david/dev/django_source/django/core/management/", line 427, in execute_from_command_line
  File "/Users/david/dev/django_source/django/core/management/", line 419, in execute
  File "/Users/david/dev/django_source/django/core/management/commands/", line 50, in run_from_argv
    super(Command, self).run_from_argv(argv)
  File "/Users/david/dev/django_source/django/core/management/", line 288, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/Users/david/dev/django_source/django/core/management/commands/", line 71, in execute
    super(Command, self).execute(*args, **options)
  File "/Users/david/dev/django_source/django/core/management/", line 337, in execute
    output = self.handle(*args, **options)
  File "/Users/david/dev/django_source/django/core/management/commands/", line 88, in handle
    failures = test_runner.run_tests(test_labels)
  File "/Users/david/dev/django_source/django/test/", line 147, in run_tests
    old_config = self.setup_databases()
  File "/Users/david/dev/django_source/django/test/", line 109, in setup_databases
    return setup_databases(self.verbosity, self.interactive, **kwargs)
  File "/Users/david/dev/django_source/django/test/", line 297, in setup_databases
    verbosity, autoclobber=not interactive)
  File "/Users/david/dev/django_source/django/db/backends/", line 368, in create_test_db
  File "/Users/david/dev/django_source/django/core/management/", line 167, in call_command
    return klass.execute(*args, **defaults)
  File "/Users/david/dev/django_source/django/core/management/", line 337, in execute
    output = self.handle(*args, **options)
  File "/Users/david/dev/django_source/django/core/management/commands/", line 145, in handle
    executor.migrate(targets, plan, fake=options.get("fake", False))
  File "/Users/david/dev/django_source/django/db/migrations/", line 60, in migrate
    self.apply_migration(migration, fake=fake)
  File "/Users/david/dev/django_source/django/db/migrations/", line 88, in apply_migration
    if self.detect_soft_applied(migration):
  File "/Users/david/dev/django_source/django/db/migrations/", line 132, in detect_soft_applied
    apps = project_state.render()
  File "/Users/david/dev/django_source/django/db/migrations/", 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 Changed 19 months ago by leftmoose

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

looking into this one possible cause might be that proxy models are ignored by makemigrations. (explicitly skipped in MigrationAutodetector._detect_changes). that means the derived state loader.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)?

comment:2 Changed 19 months ago by timo

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 19 months ago by leftmoose

what's the criteria for release blockers? this breaks foreign keys to proxy models...

comment:4 Changed 19 months ago by akaariai

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 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 Changed 19 months ago by davids

  • Owner changed from nobody to davids
  • Status changed from new to assigned

comment:6 Changed 19 months ago by andrewgodwin

  • Severity changed from Normal to 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 Changed 19 months ago by davids

  • Has patch set
  • Needs tests set
  • Patch needs improvement set

comment:8 Changed 19 months ago by loic84

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 Changed 19 months ago by davids

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 Changed 18 months ago by andrewgodwin

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 Changed 18 months ago by andrewgodwin

  • Owner changed from davids to andrewgodwin

comment:12 Changed 18 months ago by Andrew Godwin <andrew@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In c1276785f90b89c8bced606b9cbf7b1a612506e5:

Fixed #22568: Better proxy model support in migrations

comment:13 Changed 18 months ago by Andrew Godwin <andrew@…>

In a81282a512e706011747ec4cd1a990bae167edc6:

[1.7.x] Fixed #22568: Better proxy model support in migrations

Note: See TracTickets for help on using tickets.
Back to Top