Opened 16 months ago

Closed 15 months ago

Last modified 14 months ago

#22485 closed Bug (fixed)

makemigrations fails with dependencies to unmigrated apps

Reported by: apollo13 Owned by: nobody
Component: Migrations Version: master
Severity: Release blocker Keywords: migrations, unmigrated, makemigrations
Cc: andrewgodwin, charettes, RLion, loic@…, amusikal@…, dyjo, rypalmer@…, bendavis78, Naddiseo, 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 apollo13 16 months ago.

Download all attachments as: .zip

Change History (41)

Changed 16 months ago by apollo13

comment:1 Changed 16 months ago by charettes

Related to #21968 and #22397.

comment:2 Changed 16 months ago by charettes

#22488 was a duplicate.

comment:3 Changed 16 months ago by charettes

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 16 months ago by RLion

  • Cc RLion added

comment:5 Changed 16 months 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 16 months ago by charettes

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

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 16 months ago by loic84

  • Cc loic@… added

comment:10 Changed 16 months ago by amusikal

  • Cc amusikal@… added

comment:11 Changed 15 months ago by dyjo

  • Cc dyjo added

comment:12 Changed 15 months ago by rypalmer

  • Cc rypalmer@… added

comment:13 Changed 15 months ago by bendavis78

  • Cc bendavis78 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 15 months ago by Naddiseo

  • Cc Naddiseo added

comment:15 Changed 15 months ago by alexander@…

  • Cc alexander@… added

comment:16 Changed 15 months ago by kevin-brown

  • Cc kevin-brown added

comment:17 Changed 15 months ago by flisky

  • Cc flisky added

comment:18 Changed 15 months ago by andrewhayes1979

  • Cc andrewhayes1979 added

comment:19 Changed 15 months ago by hjwp

  • Cc hjwp2@… added

comment:20 Changed 15 months ago by andrewsg

  • Cc andrewsg added

comment:21 Changed 15 months ago by mjrohr330@…

  • Cc mjrohr330@… added

comment:22 Changed 15 months ago by mesemus

  • Cc miroslav.simek@… added

comment:23 Changed 15 months ago by chockey

  • Cc chockey added

comment:24 Changed 15 months 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 15 months ago by jezevec

  • Cc jezevec added

comment:26 Changed 15 months ago by Andrew Godwin <andrew@…>

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

In 8f6dff372b174e772920de6d82bd085f1a74eaf2:

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

comment:27 Changed 15 months ago by Andrew Godwin <andrew@…>

In 35c2a14a49ac3cb25dcff818b280bf0b4c290287:

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

comment:28 Changed 15 months ago by andrewgodwin

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 15 months ago by ryankask

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

Last edited 15 months ago by ryankask (previous) (diff)

comment:30 Changed 15 months ago by hjwp

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 15 months 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 15 months ago by charettes

@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 15 months ago by hjwp

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

comment:34 Changed 15 months ago by hjwp

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

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

comment:36 Changed 15 months ago by hjwp

no worries. Thanks for all the hard work!

comment:37 Changed 15 months 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 15 months ago by andrewgodwin

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

comment:39 Changed 15 months 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 14 months ago by andrewgodwin

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