Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#22485 closed Bug (fixed)

makemigrations fails with dependencies to unmigrated apps

Reported by: Florian Apolloner Owned by: nobody
Component: Migrations Version: master
Severity: Release blocker Keywords: migrations, unmigrated, makemigrations
Cc: Andrew Godwin, Simon Charette, TonyEight, loic@…, amusikal@…, JKN, rypalmer@…, Ben Davis, Richard Eames, alexander@…, Kevin Brown, flisky, andrewhayes1979, hjwp2@…, andrewsg, mjrohr330@…, miroslav.simek@…, chockey, jezevec Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Download the attached sample project and run manage.py makemigrations:

Traceback (most recent call last):
  File "./manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/home/florian/sources/django.git/django/core/management/__init__.py", line 427, in execute_from_command_line
    utility.execute()
  File "/home/florian/sources/django.git/django/core/management/__init__.py", line 419, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/florian/sources/django.git/django/core/management/base.py", line 288, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/home/florian/sources/django.git/django/core/management/base.py", line 337, in execute
    output = self.handle(*args, **options)
  File "/home/florian/sources/django.git/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/florian/sources/django.git/django/db/migrations/autodetector.py", line 33, in changes
    changes = self._detect_changes()
  File "/home/florian/sources/django.git/django/db/migrations/autodetector.py", line 50, in _detect_changes
    old_apps = self.from_state.render()
  File "/home/florian/sources/django.git/django/db/migrations/state.py", line 63, in render
    model=dangling_lookup[0]))
ValueError: Lookup failed for model referenced by field migrationtest.Test.user: auth.User

This is caused by https://github.com/django/django/commit/956bd644249337b9467c017aac4eec228dde8c5d (see the comments there) -- not sure if this is the actual cause or rather https://github.com/django/django/commit/2085f53f567f7619ecf3eab93fdacab7a5991a11 due to missing tests.

Attachments (1)

migrationtest.tar (20.0 KB) - added by Florian Apolloner 3 years ago.

Download all attachments as: .zip

Change History (41)

Changed 3 years ago by Florian Apolloner

Attachment: migrationtest.tar added

comment:1 Changed 3 years ago by Simon Charette

Related to #21968 and #22397.

comment:2 Changed 3 years ago by Simon Charette

#22488 was a duplicate.

comment:3 Changed 3 years ago by Simon Charette

Since we just rolled out 1.7b2 which non more ignore dangling related model references I guess we're going to get a lot of issues pointing here.

The planned approach here is to:

  1. Always load installed but unmigrate apps into the loader project state using fake migrations.
  2. Teach the MigrationAutodetector how to deal with those faked migration in order to avoid re-introducing #21968.

comment:4 Changed 3 years ago by TonyEight

Cc: TonyEight added

comment:5 Changed 3 years ago by loic84

@charettes I second your plan for unmigrated apps, but I'm not sure it's what's at play here, having migrations for contenttypes and auth doesn't prevent the ValueError for auth.User.

comment:6 Changed 3 years ago by Simon Charette

The ValueError for auth.User is caused by 2085f53f567f7619ecf3eab93fdacab7a5991a11 which ignores all unmigrated apps. If you revert this fix, or just pass ignore_unmigrated=False in makemigrations you'll hit a ValueError for contenttypes.ContentType instead because added fake migrations don't follow relationship to unmigrated apps.

comment:7 Changed 3 years ago by loic84

If I do makemigrations contenttypes && migrate && makemigrations auth && migrate && makemigrations business && migrate (from https://github.com/TonyEight/minimal) I get the ValueError for auth.User.

Am I missing the obvious or there shouldn't be any unmigrated apps in that case?

comment:8 Changed 3 years ago by Andrew Godwin

Running makemigrations for the apps isn't enough to get them in there early enough - contenttypes in particular loads too early in the Django process that it can't be migrated (I tried to add migrations to contrib apps and failed due to things like that and the tests).

I have a plan for fixing this, which just means fixing the projectstate creator to always include unmigrated apps when it's making migration states. There's no need for fake migrations, hopefully, and the autodetector shouldn't need any changes.

comment:9 Changed 3 years ago by loic84

Cc: loic@… added

comment:10 Changed 3 years ago by Mike Hurt

Cc: amusikal@… added

comment:11 Changed 3 years ago by JKN

Cc: JKN added

comment:12 Changed 3 years ago by rypalmer

Cc: rypalmer@… added

comment:13 Changed 3 years ago by Ben Davis

Cc: Ben Davis added

I got this same error when trying to migrate my own app when my project uses he django-taggit app. TaggedItem has a ForeignKey to ContentType, and when trying to makemigrations for one of my apps I get the following:

ValueError: Lookup failed for model referenced by field taggit.TaggedItem.content_type: contenttypes.ContentType

Is there a workaround?

comment:14 Changed 3 years ago by Richard Eames

Cc: Richard Eames added

comment:15 Changed 3 years ago by alexander@…

Cc: alexander@… added

comment:16 Changed 3 years ago by Kevin Brown

Cc: Kevin Brown added

comment:17 Changed 3 years ago by flisky

Cc: flisky added

comment:18 Changed 3 years ago by andrewhayes1979

Cc: andrewhayes1979 added

comment:19 Changed 3 years ago by Harry Percival

Cc: hjwp2@… added

comment:20 Changed 3 years ago by andrewsg

Cc: andrewsg added

comment:21 Changed 3 years ago by mjrohr330@…

Cc: mjrohr330@… added

comment:22 Changed 3 years ago by mesemus

Cc: miroslav.simek@… added

comment:23 Changed 3 years ago by chockey

Cc: chockey added

comment:24 Changed 3 years ago by melinath

Workaround for contenttypes (as described in #22518) is to make your dependencies look like this:

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

comment:25 Changed 3 years ago by jezevec

Cc: jezevec added

comment:26 Changed 3 years ago by Andrew Godwin <andrew@…>

Resolution: fixed
Status: newclosed

In 8f6dff372b174e772920de6d82bd085f1a74eaf2:

Fixed #22485: Include all unmigrated apps in project state by default.

comment:27 Changed 3 years ago by Andrew Godwin <andrew@…>

In 35c2a14a49ac3cb25dcff818b280bf0b4c290287:

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

comment:28 Changed 3 years ago by Andrew Godwin

I've fixed this, added what tests I can (we don't have a good way to test the on-disk autodetection stuff, but I've covered the bug here at least), and tested it out locally including the original test project uploaded here, some random things I made in my test project, and the original bug (#21968), as this change removes the old fix for that and adds a new one.

Further testing of this by other users would be much appreciated!

comment:29 Changed 3 years ago by Ryan Kaskel

This patch fixed the "Lookup failed" issue I was having. Thanks!

Last edited 3 years ago by Ryan Kaskel (previous) (diff)

comment:30 Changed 3 years ago by Harry Percival

I'm still seeing a problem. Repo here:

git clone https://github.com/hjwp/book-example.git   # simple app with custom user model
cd book-example/
git checkout 841635507
mkdir ../database
python3 manage.py migrate
# errors

rm accounts/migrations/0001_initial.py  # attempt to recreate migration
python3 manage.py makemigrations
python3 manage.py migrate # still errors

Hope it's of use. I think my custom user model may be a bit weird, because when I make an even simpler one (in the chapter_16 branch) everything is ok...

comment:31 Changed 3 years ago by truddick

Works for me, too. I had a bit of a speedbump figuring out how to install the patched version of 1.7, so for future reference here's what I did:

sudo pip uninstall django  # was 1.7b3
sudo pip install git+https://github.com/django/django@stable/1.7.x

comment:32 Changed 3 years ago by Simon Charette

@hjwp I can't reproduce your issue with Py2.7, Py3.4 and master, stable/1.7.x. Looking at your requirements it seems your still using 1.7b1 and not the latest master or stable/1.7.x.

comment:33 Changed 3 years ago by Harry Percival

my bad. I thought 1.7b3 included the fix. Can confirm it's fixed in stable/1.7.x from gh.

comment:34 Changed 3 years ago by Harry Percival

Will this be rolled up into an updated beta release? I'm advising my book readers to use 1.7b1 for now...

comment:35 Changed 3 years ago by Andrew Godwin

No, the next release will be RC1, as the date for that was May 1st so we're already behind.

comment:36 Changed 3 years ago by Harry Percival

no worries. Thanks for all the hard work!

comment:37 Changed 3 years ago by Anton Agestam <msn@…>

I'm still seeing this in 1.7b4.

Traceback:

(mcr-api)~/.virtualenvs/mcr-api ☕  m migrate clothing 0001
Operations to perform:
  Target specific migration: 0001_initial, from clothing
Running migrations:
  Applying clothing.0001_initial...Traceback (most recent call last):
  File "manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/Users/antonagestam/.virtualenvs/mcr-api/src/django/django/core/management/__init__.py", line 427, in execute_from_command_line
    utility.execute()
  File "/Users/antonagestam/.virtualenvs/mcr-api/src/django/django/core/management/__init__.py", line 419, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/Users/antonagestam/.virtualenvs/mcr-api/src/django/django/core/management/base.py", line 288, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/Users/antonagestam/.virtualenvs/mcr-api/src/django/django/core/management/base.py", line 337, in execute
    output = self.handle(*args, **options)
  File "/Users/antonagestam/.virtualenvs/mcr-api/src/django/django/core/management/commands/migrate.py", line 146, in handle
    executor.migrate(targets, plan, fake=options.get("fake", False))
  File "/Users/antonagestam/.virtualenvs/mcr-api/src/django/django/db/migrations/executor.py", line 62, in migrate
    self.apply_migration(migration, fake=fake)
  File "/Users/antonagestam/.virtualenvs/mcr-api/src/django/django/db/migrations/executor.py", line 90, in apply_migration
    if self.detect_soft_applied(migration):
  File "/Users/antonagestam/.virtualenvs/mcr-api/src/django/django/db/migrations/executor.py", line 134, in detect_soft_applied
    apps = project_state.render()
  File "/Users/antonagestam/.virtualenvs/mcr-api/src/django/django/db/migrations/state.py", line 86, in render
    model=lookup_model,
ValueError: Lookup failed for model referenced by field admin.LogEntry.user: user.User
(mcr-api)~/.virtualenvs/mcr-api ☕  python -c "import django; print django.get_version()"
1.7b4

Here's the migration:

# encoding: utf8
from __future__ import unicode_literals

from django.db import models, migrations


class Migration(migrations.Migration):

    dependencies = [
    ]

    operations = [
        migrations.CreateModel(
            name=b'ClothingStart',
            fields=[
                (b'id', models.AutoField(serialize=False, primary_key=True)),
                (b'clothing_id', models.IntegerField()),
                (b'created_at', models.DateTimeField()),
                (b'updated_at', models.DateTimeField()),
            ],
            options={
                'db_table': b'clothing_start',
            },
            bases=(models.Model,),
        ),
        migrations.CreateModel(
            name=b'ClothingWebshop',
            fields=[
                (b'id', models.AutoField(serialize=False, primary_key=True)),
                (b'clothing_id', models.IntegerField()),
                (b'brand_id', models.IntegerField()),
                (b'url', models.CharField(max_length=255)),
                (b'created_at', models.DateTimeField()),
                (b'updated_at', models.DateTimeField()),
                (b'star_id', models.IntegerField()),
            ],
            options={
                'db_table': b'clothing_webshop',
            },
            bases=(models.Model,),
        ),
    ]

comment:38 Changed 3 years ago by Andrew Godwin

You're not seein this bug, you're seeing #22563, which is similar.

comment:39 Changed 3 years ago by rodutSD

This seems like the closest topic.

I am seeing, what I believe to be, this bug in 1.7b4. It is not contenttype, logEntry, or user related. It doesn't appear to be dangling lookup related either. I have not manually tried the patch referenced above because I assuming that it is merged into the recent 1.7b4 release. (probably dumb)

migrate is failing for a foreign key to a class of my creation. The migration fails with this error:

File "/Users/xxx/.virtualenvs/DjangoProject/lib/python2.7/site-packages/django/db/migrations/state.py", line 86, in render
    model=lookup_model,
ValueError: Lookup failed for model referenced by field Custom_Class.thing.transaction: Another_Custom_Class.transaction


makemigrations runs fine. It creates the following dependency list which is where the problem is:

    dependencies = [
        ('Another_Custom_Class', '__first__'),        
        ('Custom_Class', '0005_mesh'),
    ]

Running migrate on this fails as above saying my reference to Another_Custom_Class.transaction from Custom_Class.thing because it can't find it.

I changed the dependency to the following and it works:

    dependencies = [
        ('Another_Custom_Class', '0003_transaction'),
        ('Custom_Class', '0005_mesh'),
    ]

I don't know how all the internals work, but it seems like it wasn't able to find the other migration because of what is going on when "_first_" is used.

This problem has happened a bunch of separate times and changing _first_ to the name of a valid migration seems to fix it. I just hate having to dig in and fix it every time.

Let me know if anyone needs more details.

comment:40 Changed 3 years ago by Andrew Godwin

Hi rodutSD,

Cross-app dependencies like this are a known issue in the current autodetector version, a rewrite is underway to fix things like this. The underlying issue is manifested in several different tickets, but if you want one to follow to see when the branch is merged go for #22605.

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