FK to proxy model breaks migrate

[leftmoose]

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>]

[leftmoose]

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)?

[leftmoose]

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

[Anssi Kääriäinen]

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.

[Andrew Godwin]

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.

[David S]

​https://github.com/django/django/pull/2645/

needs review + more tests

[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.

[David S]

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

[Andrew Godwin]

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.

[Andrew Godwin <andrew@…>]

In c1276785f90b89c8bced606b9cbf7b1a612506e5:

Fixed #22568: Better proxy model support in migrations

[Andrew Godwin <andrew@…>]

In a81282a512e706011747ec4cd1a990bae167edc6:

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