Opened 11 years ago

Closed 11 years ago

Last modified 10 years ago

#22563 closed Bug (fixed)

Migration of admin.LogEntry.user fails.

Reported by: efrinut@… Owned by: Andrew Godwin
Component: Migrations Version: 1.7-beta-2
Severity: Release blocker Keywords:
Cc: stephen.r.burrows@…, k@…, loic84, Nick Polet, chtimbo@…, murfi, jeroen@…, berto, beathan, no, jezevec, Selwin Ong, Ben Davis, lemuelf@…, ludovic.legendart@…, joar-5m, dbinetti@…, Giacomo Graziosi Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I am not 100% sure if this will help with reproducing the issue.

I've set up a postgresql databasae with app and model config which is below.

When I try to migrate I get an error:

ValueError: Lookup failed for model referenced by field admin.LogEntry.user: accounts.User

To fix this error error I did these steps:

  1. Commented out django.contrib.admin and admin in urls.py
  2. Applied migrations
  3. Uncommented django.contrib.admin and urls.py
DJANGO_APPS = (
    'django.contrib.contenttypes',
    'django.contrib.admin',
    'django.contrib.auth',
    'django.contrib.sessions',
    'django.contrib.sites',
    'django.contrib.messages',
    'django.contrib.staticfiles',
    'django.contrib.admindocs',
    'django.contrib.humanize',
    'django.contrib.sitemaps',
    'django.contrib.flatpages',
)

EXTERNAL_APPS = (
    'haystack',
    'storages',
    'pipeline',
    'endless_pagination',
    'easy_thumbnails',
    'mptt',
    'rest_framework',
    'crispy_forms',
    'crispy_forms_foundation',
    'taggit',
)


SITE_APPS = (
    'accounts',
    'core',
    'images',
    'streams',
    'links',
    'entries',
)
AUTH_USER_MODEL = 'accounts.User'

INSTALLED_APPS = DJANGO_APPS + EXTERNAL_APPS + SITE_APPS
from django.db import models
from django.contrib.auth.models import AbstractUser
from django.core.urlresolvers import reverse


class User(AbstractUser):
    description = models.TextField(null=True, blank=True, verbose_name=u"Write something about yourself")
    link_blank = models.BooleanField(default=False, verbose_name=u"Open links in new window")
    link_direct = models.BooleanField(default=False, verbose_name=u"Skip directly to the link")

    def get_absolute_url(self):
        return reverse("user:detail", kwargs={'pk': self.pk})

    def get_edit_url(self):
        return reverse("user:edit", kwargs={'pk': self.pk})

WARNINGS:
?: (1_6.W001) Some project unittests may not execute as expected.
        HINT: Django 1.6 introduced a new default test runner. It looks like thi
s project was generated using Django 1.5 or earlier. You should ensure your test
s are all running & behaving as expected. See https://docs.djangoproject.com/en/
dev/releases/1.6/#new-test-runner for more information.
Operations to perform:
  Synchronize unmigrated apps: pipeline, mptt, sessions, admin, debug_toolbar, s
ites, auth, easy_thumbnails, sitemaps, endless_pagination, contenttypes, flatpag
es, images, rest_framework, haystack, storages, crispy_forms, crispy_forms_found
ation
  Apply all migrations: streams, accounts, taggit
Synchronizing apps without migrations:
  Creating tables...
  Installing custom SQL...
  Installing indexes...
Running migrations:
  Applying taggit.0001_initial...Traceback (most recent call last):
  File "local_manage.py", line 22, in <module>
    execute_from_command_line(sys.argv)
  File "D:\projekty\myinit\env\lib\site-packages\django\core\management\__init__
.py", line 427, in execute_from_command_line
    utility.execute()
  File "D:\projekty\myinit\env\lib\site-packages\django\core\management\__init__
.py", line 419, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "D:\projekty\myinit\env\lib\site-packages\django\core\management\base.py"
, line 288, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "D:\projekty\myinit\env\lib\site-packages\django\core\management\base.py"
, line 337, in execute
    output = self.handle(*args, **options)
  File "D:\projekty\myinit\env\lib\site-packages\django\core\management\commands
\migrate.py", line 146, in handle
    executor.migrate(targets, plan, fake=options.get("fake", False))
  File "D:\projekty\myinit\env\lib\site-packages\django\db\migrations\executor.p
y", line 60, in migrate
    self.apply_migration(migration, fake=fake)
  File "D:\projekty\myinit\env\lib\site-packages\django\db\migrations\executor.p
y", line 88, in apply_migration
    if self.detect_soft_applied(migration):
  File "D:\projekty\myinit\env\lib\site-packages\django\db\migrations\executor.p
y", line 132, in detect_soft_applied
    apps = project_state.render()
  File "D:\projekty\myinit\env\lib\site-packages\django\db\migrations\state.py",
 line 80, in render
    model=lookup_model
ValueError: Lookup failed for model referenced by field admin.LogEntry.user: accounts.User

Change History (66)

comment:1 by Tim Graham, 11 years ago

Triage Stage: UnreviewedAccepted
Version: master1.7-beta-2

I can reproduce; also reported in #22535.

comment:2 by Stephen Burrows, 11 years ago

I am also having this problem. Here's my output from git bisect:

35c2a14a49ac3cb25dcff818b280bf0b4c290287 is the first bad commit
commit 35c2a14a49ac3cb25dcff818b280bf0b4c290287
Author: Andrew Godwin <andrew@aeracode.org>
Date:   Wed Apr 30 12:25:12 2014 -0700

    [1.7.x] Fixed #22485: Include all unmigrated apps in project state by default.

:040000 040000 b585f518764b3f89f8dfc01ad7005d6109412aaa a831ee56a0017ea4349dabe4d86738a5dc0b642c M	django
:040000 040000 dc9cef91bb08da4d450af28c7b27972642caa09a 556c29640010d8c30c742871d48164dc42ed30a4 M	tests

So it looks like this was caused by the fix for #22485 :-p

comment:3 by Stephen Burrows, 11 years ago

Regenerating migrations doesn't help.

comment:4 by Stephen Burrows, 11 years ago

Cc: stephen.r.burrows@… added

comment:5 by Kevin Christopher Henry, 11 years ago

Cc: k@… added

comment:6 by loic84, 11 years ago

Cc: loic84 added

I couldn't reproduce, any tips?

comment:7 by chtimbo, 11 years ago

I can reproduce it too, with Django 1.7.x checked out a few hours ago, with a custom User model inheriting directly from Abstract User.
Trying to run tests with manage.py test also raises the same exception, and the same workaround works for both migrations and tests.

comment:8 by Tim Graham, 11 years ago

To reproduce I used the tutorial.

  1. Create an initial migration for polls and run it. Then add the user in the description of this ticket to the bottom of poll/models.py and AUTH_USER_MODEL = 'polls.User' to settings.py.
  1. Try to create the migration:
    $ python manage.py makemigrations polls
    Traceback (most recent call last):
      File "manage.py", line 10, in <module>
        execute_from_command_line(sys.argv)
      File "/home/tim/code/django/django/core/management/__init__.py", line 427, in execute_from_command_line
        utility.execute()
      File "/home/tim/code/django/django/core/management/__init__.py", line 419, in execute
        self.fetch_command(subcommand).run_from_argv(self.argv)
      File "/home/tim/code/django/django/core/management/base.py", line 288, in run_from_argv
        self.execute(*args, **options.__dict__)
      File "/home/tim/code/django/django/core/management/base.py", line 337, in execute
        output = self.handle(*args, **options)
      File "/home/tim/code/django/django/core/management/commands/makemigrations.py", line 99, in handle
        changes = autodetector.changes(graph=loader.graph, trim_to_apps=app_labels or None)
      File "/home/tim/code/django/django/db/migrations/autodetector.py", line 36, in changes
        changes = self._detect_changes()
      File "/home/tim/code/django/django/db/migrations/autodetector.py", line 53, in _detect_changes
        old_apps = self.from_state.render()
      File "/home/tim/code/django/django/db/migrations/state.py", line 83, in render
        model=lookup_model
    ValueError: Lookup failed for model referenced by field admin.LogEntry.user: polls.User
    

comment:9 by Nick Polet, 11 years ago

Cc: Nick Polet added

comment:10 by chtimbo@…, 11 years ago

Cc: chtimbo@… added

comment:11 by murfi, 11 years ago

Cc: murfi added

comment:12 by Andrew Godwin, 11 years ago

Owner: changed from nobody to Andrew Godwin
Status: newassigned

comment:13 by Andrew Godwin, 11 years ago

This is actually expected; migrations don't supporting changing AUTH_USER_MODEL (or any swappable model) after you've made an initial migration, and this has always been the intention, but before that commit we were leaking the state enough that it worked (though it would have had bugs).

The key problem here is that AUTH_USER_MODEL directly affects ForeignKeys and relationships but exists outside of the model history - and because it's not versioned, changing it means the migrations get all broken. The fix is to delete all migrations and restart from scratch if you change AUTH_USER_MODEL; there's no other way of guaranteeing your database is correct.

I'll update the error message to detect if the missing model is AUTH_USER_MODEL and tell this to the user.

comment:14 by Andrew Godwin <andrew@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In fc974313b85da932a70f1f993b33207d78d31831:

Fixed #22563: Better error message when trying to change AUTH_USER_MODEL

You're not allowed to do this after you've made migrations; see ticket
for more details.

comment:15 by Andrew Godwin <andrew@…>, 11 years ago

In f67433e74b06bbeb9b6bba0a9db5fb33512bdb43:

[1.7.x] Fixed #22563: Better error message when trying to change AUTH_USER_MODEL

You're not allowed to do this after you've made migrations; see ticket
for more details.

comment:16 by Stephen Burrows, 11 years ago

Resolution: fixed
Status: closednew

I get this error even if I *haven't* changed the value of AUTH_USER_MODEL. Even if I delete and regenerate the migrations.

(Though I can't actually test the raw state of this bug due to #22325. In order to apply the migrations, I have to delete all the migrations.swappable_dependency(settings.AUTH_USER_MODEL) declarations.)

comment:17 by Andrew Godwin, 11 years ago

Ah yes, you're right, it does occur even from scratch. Not sure how we can support that; I'll have to leave this one and ponder it. Swappable models are a right PITA and we might have to exclude them from migrations or something if it gets really bad.

comment:18 by Ben Davis, 11 years ago

At the very least this needs a regression test. The fact that tests are passing on such a broad use case (eg, following the tutorial) isn't good.

comment:19 by Andrew Godwin <andrew@…>, 11 years ago

Resolution: fixed
Status: newclosed

In 5182efce8d73ec552a1d7f3e86a24369bb261044:

Fixed #22563: Ignore AUTH_USER_MODEL errors in from_state

comment:20 by Andrew Godwin <andrew@…>, 11 years ago

In 4535dedc42b90b8f75576e48a12e2e63d4e93048:

[1.7.x] Fixed #22563: Ignore AUTH_USER_MODEL errors in from_state

comment:21 by Andrew Godwin, 11 years ago

OK, I've committed a patch that suppresses the error in that one case (where we know it's safe to do so), and I can now switch AUTH_USER_MODEL even midway through a migration set without errors. Would appreciate confirmation/further reports.

comment:22 by berto, 11 years ago

I just ran into this issue as well. I did two things in order to fix it.

First, the problem appears when performing a clean install and attempting to run manage.py migrate on an app that defines the AUTH_USER_MODEL. While developing this app I created a number of models including the AssociationUser model:

  • AssociationUser
  • Association
  • Address
  • ...

When I ran manage.py makemigrations the migrations were created in this order:

Migrations for 'association':
  0001_initial.py:
    - Create model Address
    - Create model Association
  0002_associationuser.py:
    - Create model AssociationUser
[...]

This fails with the error from this issue: ValueError: Lookup failed for model referenced by field admin.LogEntry.user: accounts.User. Manually swapping the initial and second migration's operations so that the user model is created first solved this issue.

Second, I ran into this problem with a 3rd party app that defines migrations (taggit, https://github.com/alex/django-taggit). This also fails with the ValueError specified above, but is tricker to solve. My solution at the moment is to vendor the app by adding its code to my repo and adding a dependency to the app containing the user model, i.e:

    dependencies = [
        ('contenttypes', '__first__'),
        (b'association', b'0001_initial'),
    ]

While I don't know the internals enough to make an educated suggestion, could the migrations framework include a dependency function to say, "this migration requires the AUTH_USER_MODEL to exist". Alternatively, maybe some initialization logic to migrate AUTH_USER_MODEL as early as possible.

Thanks!

comment:23 by Andrew Godwin, 11 years ago

berto: I fixed this ticket 30 minutes before you opened it; are these errors being reported after my fix was applied, or did the timing just work out badly?

As for the third-party migrations on user, this is why migrations has the following pattern for depending on the user model:

    dependencies = [
        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
    ]

Any migration that references the User model should have this auto-added.

comment:24 by berto, 11 years ago

Andrew: I have re-tested the two scenarios in my update with commit 31eac71a760b5f30796abfb0b18c3f7578b93e84.

Adding swappable_dependency to the dependencies list solves the third-party issue.

For the other problem, I deleted all migrations from my app and re-ran manage.py makemigrations. AUTH_USER_MODEL is still created as the second migration, thus applying the migrations fails with the ValueError from this issue. Swapping the first two migrations as in my previous comment allows the migrations to be applied.

Just for kicks I added swappable_dependency to the initial migration in my app, but that does not fix the problem; I have to swap the first two migrations for it to work.

Thanks!

comment:25 by berto, 11 years ago

Andrew: please take a look at the thread at https://github.com/alex/django-taggit/pull/235

While swappable_dependency does fix the third-party issue, the third party app does not utilize the user model. As @apollo13 states in the thread, this may be a bug in the migrations system.

comment:26 by Jeroen Dekkers, 11 years ago

Cc: jeroen@… added
Resolution: fixed
Status: closednew

Reopening the ticket as it is not completely fixed yet. I run into the same problem as berto. I tested the latest commit in the stable/1.7.x branch (9d0ebceb324ebb13b6d859153f95b2ef0c7c326b). To reproduce, create the following models.py:

from django.db import models
from django.contrib.auth.models import AbstractUser


class MyModel(models.Model):
    pass


class MyUser(AbstractUser):
    ref = models.ForeignKey('MyModel')

Change the user model to MyUser:

AUTH_USER_MODEL = 'myapp.MyUser'

Run makemigrations, you can see that the migration for MyUser is created second:

Migrations for 'myapp':
  0001_initial.py:
    - Create model MyModel
  0002_myuser.py:
    - Create model MyUser

And migrating fails:

Operations to perform:
  Synchronize unmigrated apps: admin, contenttypes, auth, sessions
  Apply all migrations: myapp
Synchronizing apps without migrations:
  Creating tables...
  Installing custom SQL...
  Installing indexes...
Running migrations:
  Applying myapp.0001_initial...Traceback (most recent call last):
  File "./manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/home/jeroen/github/django/django/core/management/__init__.py", line 427, in execute_from_command_line
    utility.execute()
  File "/home/jeroen/github/django/django/core/management/__init__.py", line 419, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/jeroen/github/django/django/core/management/base.py", line 288, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/home/jeroen/github/django/django/core/management/base.py", line 337, in execute
    output = self.handle(*args, **options)
  File "/home/jeroen/github/django/django/core/management/commands/migrate.py", line 146, in handle
    executor.migrate(targets, plan, fake=options.get("fake", False))
  File "/home/jeroen/github/django/django/db/migrations/executor.py", line 62, in migrate
    self.apply_migration(migration, fake=fake)
  File "/home/jeroen/github/django/django/db/migrations/executor.py", line 90, in apply_migration
    if self.detect_soft_applied(migration):
  File "/home/jeroen/github/django/django/db/migrations/executor.py", line 134, in detect_soft_applied
    apps = project_state.render()
  File "/home/jeroen/github/django/django/db/migrations/state.py", line 86, in render
    model=lookup_model,
ValueError: Lookup failed for model referenced by field admin.LogEntry.user: myapp.MyUser

Simply swapping the migrations as berto suggested won't work for this case because it can't create MyUser when MyModel doesn't exist yet. A workaround is to first create MyUser without ForeignKey and then create MyModel and add the ForeignKey.

comment:27 by Jeroen Dekkers, 11 years ago

Another thing I just noticed: while the migration of the app works when the User model is added in the first migration, running "migrate" fails with the same error when there are multiple apps to migrate and the app with the User model isn't the first one to run.

comment:28 by berto, 11 years ago

Cc: berto added

comment:29 by berto, 11 years ago

@dekkers, in your case does the migration succeed if you manually merge the two migrations into one?

comment:30 by beathan, 11 years ago

Cc: beathan added

comment:31 by no, 11 years ago

Cc: no added

comment:32 by Nick Polet, 11 years ago

I can't run tests as I get the "ValueError: Lookup failed for model referenced by field admin.LogEntry.user...." error when the test system is trying to create the test database (unless I comment out the admin system in settings.py and urls.py). Has there been any progress on this bug? It's pretty much a show stopper at the moment.

Last edited 11 years ago by Nick Polet (previous) (diff)

comment:33 by Andrew Godwin, 11 years ago

This bug is much more limited now to dependency management and so will probably be fixed along with #22470 in the next few weeks. We don't treat bugs in pre-release versions as "show stoppers", as you're not supposed to be doing production code on them yet!

That said, you can probably fix the bug by manually re-ordering your migration operations as described above.

comment:34 by jezevec, 11 years ago

Cc: jezevec added

in reply to:  20 comment:35 by rockallite.wulf@…, 11 years ago

Replying to Andrew Godwin <andrew@…>:

In 4535dedc42b90b8f75576e48a12e2e63d4e93048:

[1.7.x] Fixed #22563: Ignore AUTH_USER_MODEL errors in from_state

Hi. I still get the error using Django 1.7b4 which contains the fix.

I'm in the same situation with @dekkers. My custom user model myapp.User contains a foreign key to another model. When I run the migration, the error is raised.

I looked into the traceback and see django.db.migrations.operations.models.CreateModel was invoking to_state.render() without the ignore_swappable=True argument, which caused the problem.

I manually changed it to to_state.render(ignore_swappable=True), then the migration was successfully executed.

However, ./manage.py test fails with the same error: ValueError: Lookup failed for model referenced by field admin.LogEntry.user: myapp.User, and this time in django.db.migrations.executor in detect_soft_applied...

Perhaps a better fix is to change the signature of django.db.migrations.state.ProjectState.render to be invoked with ignore_swappable=True by default. I've looked into the code:

def render(self, include_real=None, ignore_swappable=False):
    # ... Other code omitted ...
    if "%s.%s" % (lookup_model[0], lookup_model[1]) == settings.AUTH_USER_MODEL and ignore_swappable:
        continue

This is the only occurrence of the ignore_swappable parameter in the method body. So I think it isn't so bad to change ignore_swappable parameter to True by default.

comment:36 by Selwin Ong, 11 years ago

Cc: Selwin Ong added

comment:37 by Ben Davis, 11 years ago

I'm also seeing the same error as comment:20 with migrate:

Lookup failed for model referenced by field admin.LogEntryUser

on 1.7b4 and master when using a custom AUTH_USER_MODEL.

comment:38 by Camilo Nova, 11 years ago

A workaround is to delete the django_admin_log table and then recreate it again.

$ ./manage.py dbshell

=> drop table django_admin_log cascade ;
DROP TABLE

$ ./manage.py migrate
Synchronizing apps without migrations:
  Creating tables...
    Creating table django_admin_log
  Installing custom SQL...
...

Hope to see this issue fixed soon.

comment:39 by Ben Davis, 11 years ago

Cc: Ben Davis added

comment:40 by Lemuel Formacil, 11 years ago

Cc: lemuelf@… added

comment:41 by TonyEight, 11 years ago

Cc: ludovic.legendart@… added

comment:42 by joar-5m, 11 years ago

Cc: joar-5m added

comment:43 by David Binetti <dbinetti@…>, 11 years ago

Cc: dbinetti@… added

comment:44 by Matthew Schinckel, 11 years ago

I'm seeing similar behaviour: it appears to be when the app that contains the AUTH_USER_MODEL appears in INSTALLED_APPS _after_ 'django.contrib.admin'.

That is using 1.7b4, and the current git checkout.

Making the change suggested in https://code.djangoproject.com/ticket/22563?cnum_edit=44#comment:35 makes it work (change to ignore_swappable=True).

The suggestion in comment 38 (deleting the table) did _not_ work.

Version 2, edited 11 years ago by Matthew Schinckel (previous) (next) (diff)

comment:45 by Stephen Burrows, 11 years ago

FTR, I'm getting this with the app containing the AUTH_USER_MODEL coming before django.contrib.admin

in reply to:  description comment:46 by Giacomo Graziosi, 11 years ago

Cc: Giacomo Graziosi added

comment:47 by Andrew Godwin, 11 years ago

I've just landed a commit that fixes a bug with a similar error message (in https://github.com/django/django/commit/1fe941adb0570a58cc5d2eb372611ade8d0e8776) but I suspect it's not this bug - could someone restest to make sure?

comment:48 by Andrew Godwin, 11 years ago

Or, indeed, it may have been fixed by the autodetector rewrite; it's just almost midnight here and I need to sleep, so someone else testing would be great.

in reply to:  48 comment:49 by Vidir Valberg Gudmundsson, 11 years ago

Replying to andrewgodwin:

Or, indeed, it may have been fixed by the autodetector rewrite; it's just almost midnight here and I need to sleep, so someone else testing would be great.

Sadly not as I can tell.

But I discovered something odd. If you only have the following in your models.py:

from django.db import models
from django.contrib.auth.models import AbstractUser


class UserModel(AbstractUser):
    more_info = models.CharField(max_length=100)

It works just fine:

$ ./manage.py makemigrations           
Migrations for 'testapp':
  0001_initial.py:
    - Create model UserModel

$ ./manage.py migrate       
Operations to perform:
  Synchronize unmigrated apps: sessions, auth, contenttypes, admin
  Apply all migrations: testapp
Synchronizing apps without migrations:
  Creating tables...
    Creating table django_admin_log
    Creating table auth_permission
    Creating table auth_group_permissions
    Creating table auth_group
    Creating table django_content_type
    Creating table django_session
  Installing custom SQL...
  Installing indexes...
Running migrations:
  Applying testapp.0001_initial... OK

You have installed Django's auth system, and don't have any superusers defined.
Would you like to create one now? (yes/no): yes
Username: test
Email address: test@test.com
Password: 
Password (again): 
Superuser created successfully.

But if there is just one other model in models.py, like this:

from django.db import models

from django.contrib.auth.models import AbstractUser


class Foo(models.Model):
    bar = models.CharField(max_length=100)


class UserModel(AbstractUser):
    more_info = models.CharField(max_length=100)

It fails:

> ./manage.py makemigrations           
Migrations for 'testapp':
  0001_initial.py:
    - Create model Foo
    - Create model UserModel

> ./manage.py migrate       
Operations to perform:
  Synchronize unmigrated apps: contenttypes, admin, auth, sessions
  Apply all migrations: testapp
Synchronizing apps without migrations:
  Creating tables...
    Creating table django_admin_log
    Creating table auth_permission
    Creating table auth_group_permissions
    Creating table auth_group
    Creating table django_content_type
    Creating table django_session
  Installing custom SQL...
  Installing indexes...
Running migrations:
  Applying testapp.0001_initial...Traceback (most recent call last):
  File "/home/valberg/.virtualenvs/django/lib/python3.3/site-packages/django/apps/config.py", line 152, in get_model
    return self.models[model_name.lower()]
KeyError: 'usermodel'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/valberg/.virtualenvs/django/lib/python3.3/site-packages/django/db/migrations/state.py", line 76, in render
    model = self.apps.get_model(lookup_model[0], lookup_model[1])
  File "/home/valberg/.virtualenvs/django/lib/python3.3/site-packages/django/apps/registry.py", line 190, in get_model
    return self.get_app_config(app_label).get_model(model_name.lower())
  File "/home/valberg/.virtualenvs/django/lib/python3.3/site-packages/django/apps/config.py", line 155, in get_model
    "App '%s' doesn't have a '%s' model." % (self.label, model_name))
LookupError: App 'testapp' doesn't have a 'usermodel' model.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "./manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/home/valberg/.virtualenvs/django/lib/python3.3/site-packages/django/core/management/__init__.py", line 385, in execute_from_command_line
    utility.execute()
  File "/home/valberg/.virtualenvs/django/lib/python3.3/site-packages/django/core/management/__init__.py", line 377, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/valberg/.virtualenvs/django/lib/python3.3/site-packages/django/core/management/base.py", line 288, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/home/valberg/.virtualenvs/django/lib/python3.3/site-packages/django/core/management/base.py", line 337, in execute
    output = self.handle(*args, **options)
  File "/home/valberg/.virtualenvs/django/lib/python3.3/site-packages/django/core/management/commands/migrate.py", line 146, in handle
    executor.migrate(targets, plan, fake=options.get("fake", False))
  File "/home/valberg/.virtualenvs/django/lib/python3.3/site-packages/django/db/migrations/executor.py", line 62, in migrate
    self.apply_migration(migration, fake=fake)
  File "/home/valberg/.virtualenvs/django/lib/python3.3/site-packages/django/db/migrations/executor.py", line 96, in apply_migration
    migration.apply(project_state, schema_editor)
  File "/home/valberg/.virtualenvs/django/lib/python3.3/site-packages/django/db/migrations/migration.py", line 107, in apply
    operation.database_forwards(self.app_label, schema_editor, project_state, new_state)
  File "/home/valberg/.virtualenvs/django/lib/python3.3/site-packages/django/db/migrations/operations/models.py", line 33, in database_forwards
    apps = to_state.render()
  File "/home/valberg/.virtualenvs/django/lib/python3.3/site-packages/django/db/migrations/state.py", line 86, in render
    model=lookup_model,
ValueError: Lookup failed for model referenced by field admin.LogEntry.user: testapp.UserModel

(The above is run with the stable/1.7.x branch at b236a5581898aca7bff082a6d642bf457278c658 so it should include the autodetector rewrite.)

comment:50 by Ben Davis, 11 years ago

I'm also still seeing the issue on 1.7.x. I have to switch to 1.7.b2 to do migrations.

comment:51 by Ben Davis, 11 years ago

I decided to dig in for a few hours and see if I could find the cause to this. I've found what's causing this bug.

When manage.py migrate is called, during the MigrationExecutor.detect_soft_applied() method, A new ProjectState is created. This project state accepts a list of "real apps" to use for creating a fake AppConfig registry (kind of like a mock for django.apps.apps). In this case, it is passed a list of *unmigrated apps* (apps that do not yet have migration modules).

The ProjectState then attempts to render all models in this list of apps. If one of these has a ForeignKey to a model that has not yet been rendered, the target model is put into a "pending_lookups" list so that it can be resolved later on. Once ProjectState finishes rendering all of the models in its real_apps list, it moves on the the "pending_lookups" list so that it can call do_pending_lookups for those target models.

Now, here's the bug: If a model being rendered has a ForeignKey to a model from a *migrated* app (ie, an app with a migrations module), the FK lookup will fail because it doesn't exist in the ProjectState's fake AppConfg instance. The FK Lookup is defferred until later, but ultimately fails because the app is never added to the fake AppConfig.

I'm working on a test case right now and will attach it soon.

comment:52 by Ben Davis, 11 years ago

Ok, so I just realized the issue I was having is probably different from the above issue above, but it might be similar. I'm throwing in the towel for now, hopefully I didn't confuse things too much :-/

comment:53 by Andrew Godwin, 11 years ago

Thanks for looking into this issue, though, bendavis78! If you do find a bug at the end of the (figurative) rainbow, then please file it!

(That might be the issue left; there's so many error paths that end in errors like this it can be hard to tell. South had this with the infamous "string has no attribute _meta" bugs for about 5 years)

comment:54 by Ben Davis, 11 years ago

Yeah, so I realized the situation I described above would only happen in the following case: I have three apps, A, B, and C. Apps A and B have unapplied initial migrations, while C has no migrations yet. If C has a FK to B, and you try to run migrate for A, that's when the exception occurs. In my case I just forgot to make migrations for C, so I'm not sure yet if this is something that needs fixing or just needs a better error message. I suppose it needs fixing since C could theoretically be faked in order for A to go through.

But, it did get me thinking about the issue with admin.LogEntry, and it could be the same problem. Since admin.LogEntry has a FK to the swappable model, if the app with swappable model has an initial unapplied migration, and you're trying to apply the initial migration with a different app, I imagine you could end up with the same situation. I'd need to write out proper tests for both cases though.

comment:55 by Ben Davis, 11 years ago

I've created a new ticket, with a failing test case, which covers the issue I described above: #22823

comment:56 by Andrew Godwin, 11 years ago

Status: newassigned

comment:57 by Andrew Godwin, 11 years ago

I've fixed #22823, which should fix at least some of the issues with this bug, but I would expect admin to still fail to syncdb as it'll be trying to FK a table that doesn't exist yet (syncdb emulation phase comes before migrations).

I think at some level we'll have to say that ForeignKeys from unmigrated apps to migrated apps are just not allowed, or that if you do them, you realise you'll need to manually migrate the depended-on app first before running general migrate. Without migrations and dependencies in place we can't determine what order to make things in or if there's circular dependencies.

I suggest we add migrations to django.contrib.admin, which I think was one of the less troublesome contrib apps to add migrations to in my testing, and then document that ForeignKeys/M2Ms from unmigrated apps to migrated ones are heavily discourages, but that you can make it work with extra work.

comment:58 by Ben Davis, 11 years ago

@andrewgodwin, I've just determined that this ticket is caused by the same as #22823. The traceback posted by comment:49 is slightly different, but still fails for the same reason. This bug will occur for any unmigrated app that has a foreign key to a swappable model. So, I'm not sure we can avoid having FK's from unmigrated app to migrated apps.

I'll try out the patch on my various test environments.

comment:59 by Ben Davis, 11 years ago

FYI: This is a similar test case for the AdminLog issue: https://github.com/bendavis78/django/commit/913e5bcb19e331d56f8bc44a35253c01a5f61f9a

Note: I branched it from #22823 and just commented out that test

comment:60 by Stephen Burrows, 11 years ago

There was a (undocumented) way to declare dependency on non-migrating apps ('auth', '__first__' IIRC) - but it looks like django migrations don't support reverse dependencies (i.e. declaring that a third-party app depends on your app) so I suppose that wouldn't help even if it were still a thing.

Last edited 11 years ago by Stephen Burrows (previous) (diff)

comment:61 by Andrew Godwin, 11 years ago

Yeah, the swappable models thing just means that this only appears sometimes (when the swappable model is in a migrated app).

There's literally no way around this I can think of other than requiring migrated apps to only be depended on by migrated apps. If you're using third party apps pointing swappable models you'll just have to have your user in an unmigrated app or make sure all third-party apps have migrations (a future we'll reach eventually anyway, as Django 1.9 or 2.0 will require migrations for every app).

(Django migrations does have reverse dependencies, actually, but they're a) discouraged and b) still only work inside the set of migrated apps; you can't depend on an unmigrated app as there's no migration nodes to depend on)

comment:62 by Andrew Godwin <andrew@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In edd9f8a7b2beb0fe2f83a9e50157c45364753121:

Fixed #22563: Added migration to admin, fixed a few more loader issues.

comment:63 by Andrew Godwin <andrew@…>, 11 years ago

In c903543127e642e619213cdefda9cea15df09c16:

[1.7.x] Fixed #22563: Added migration to admin, fixed a few more loader issues.

comment:64 by Andrew Godwin, 11 years ago

There's also a documentation fix as part of #22660 to say "don't have unmigrated apps depending on migrated ones". We've solved this in core by giving migrations to admin.

comment:65 by Tim Graham <timograham@…>, 10 years ago

In fca866763acb6b3414c20ca3772b94cb5d111733:

Added test for an intermediate swappable model change in migration state.

refs #22563

comment:66 by Tim Graham <timograham@…>, 10 years ago

In d49b5851b45cee5471aef37d2b301e3084f94e08:

[1.7.x] Added test for an intermediate swappable model change in migration state.

refs #22563

Backport of fca866763acb6b3414c20ca3772b94cb5d111733 from master

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