Opened 4 years ago

Closed 4 years ago

#22824 closed Bug (fixed)

Unhandled KeyError during makemigrations

Reported by: Ben Davis Owned by: nobody
Component: Migrations Version: master
Severity: Release blocker Keywords:
Cc: django@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Haven't figured out exactly what's causing this one, but I'm putting the exception here in case it rings any bells.

Traceback (most recent call last):
  File "/home/ben/Projects/myproject/src/manage.py", line 20, in <module>
    execute_from_command_line(sys.argv)
  File "/home/ben/Projects/myproject/env/src/django/django/core/management/__init__.py", line 385, in execute_from_command_line
    utility.execute()
  File "/home/ben/Projects/myproject/env/src/django/django/core/management/__init__.py", line 377, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/ben/Projects/myproject/env/src/django/django/core/management/base.py", line 288, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/home/ben/Projects/myproject/env/src/django/django/core/management/base.py", line 337, in execute
    output = self.handle(*args, **options)
  File "/home/ben/Projects/myproject/env/src/django/django/core/management/commands/makemigrations.py", line 100, in handle
    convert_apps=app_labels or None,
  File "/home/ben/Projects/myproject/env/src/django/django/db/migrations/autodetector.py", line 37, in changes
    changes = self._detect_changes(convert_apps)
  File "/home/ben/Projects/myproject/env/src/django/django/db/migrations/autodetector.py", line 208, in _detect_changes
    for other_operation in self.generated_operations[dep[0]]:
KeyError: 'myapp'

I'm seeing if I can reproduce this in a test case as well.

Attachments (1)

autodetector.diff (762 bytes) - added by Ben Davis 4 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 4 years ago by Ben Davis

Ok, this seems to occur when, for example, you have three unmigrated apps:

# unmigrated_a/models.py
class ModelA(models.Model):
    somefield = models.CharField()

# unmigrated_b/models.py
from unmigrated_c.models import ModelC

class ModelB(models.Model):
    somefield = models.CharField()
    somefk = models.ForeignKey(ModelC)
# unmigrated_c/models.py
class ModelC(models.Model):
    somefield = models.CharField()

Running makemigrations unmigrated_a works fine. Running makemigrations unmigrated_b fails with the KeyError

The same eror occurs even if you run makemigrations unmigrated_c before makemigrations unmigrated_b -- the command still fails for unmigrated_b.

Test case here: https://github.com/bendavis78/django/commit/1996fbfa963ac853ee62c9e673da35257e90556d

comment:2 Changed 4 years ago by Richard Eames

Cc: django@… added

If you delete all the migrations from all apps, and then run makemigrations unmigrated_c unmigrated_b unmigrated_a does it work then? (If so, I was able to reproduce this on Monday)

comment:3 Changed 4 years ago by Ben Davis

@Naddiseo, yeah looks like putting them in order C, B, A has the same effect.

comment:4 Changed 4 years ago by Ben Davis

Severity: NormalRelease blocker

comment:5 Changed 4 years ago by Ben Davis

I've made a small change which passes the test case, but I'm getting a different error in my main project, so I'm not sure that if it's the right fix and there's another bug, or if this is the wrong fix. This dependency autodetector loop is not easy to wrap my head around.

  • django/db/migrations/autodetector.py

    diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py
    index 6ed6edc..7276773 100644
    a b class MigrationAutodetector(object): 
    205205                        elif dep[0] != app_label:
    206206                            # External app dependency. See if it's not yet
    207207                            # satisfied.
    208                             for other_operation in self.generated_operations[dep[0]]:
    209                                 if self.check_dependency(other_operation, dep):
    210                                     deps_satisfied = False
    211                                     break
     208                            if self.generated_operations.get(dep[0]):
     209                                for other_operation in self.generated_operations[dep[0]]:
     210                                    if self.check_dependency(other_operation, dep):
     211                                        deps_satisfied = False
     212                                        break
    212213                            if not deps_satisfied:
    213214                                break
    214215                            else:

comment:6 Changed 4 years ago by Ben Davis

Ok, so ignore the above diff, definitely not the right answer.

For my specific case I was able to avoid the error by creating all my migrations in one command, eg: ./manage.py migrate unmigrated_a unmigrated_b and unmigrated_c. I think this is still a bug that needs fixing, though.

comment:7 in reply to:  6 Changed 4 years ago by anonymous

Replying to bendavis78:

For my specific case I was able to avoid the error by creating all my migrations in one command, eg: ./manage.py migrate unmigrated_a unmigrated_b and unmigrated_c. I think this is still a bug that needs fixing, though.

That's what I meant by my earlier comment; I found that it works if you include all the unmigrated apps in one command. So, I guess this is the bug I hit on Monday. :)

comment:8 Changed 4 years ago by Baptiste Mispelon

Triage Stage: UnreviewedAccepted

Hi,

I can reproduce the issue with only two apps (in my example, they're called bug22824 and bug22824_):

# bug2284/models.py
from django.db import models

class Foo(models.Model):
    pass

# bug22824_/models.py
from django.db import models

class Bar(models.Model):
    foo = models.ForeignKey('bug22824.Foo')

Running makemigrations bug22824 bug22824_ works (so does makemigrations bug22824_ bug22824)
Running makemigrations bug22824_ then makemigrations bug22824 works (the first makemigrations creates migrations for both apps and the second does nothing)
Running makemigrations bug22824 then makemigrations bug22824_ breaks (on the second migration) with the reported KeyError.

Thanks.

Changed 4 years ago by Ben Davis

Attachment: autodetector.diff added

comment:9 Changed 4 years ago by Ben Davis

I ran into this issue again and looked into it. The common thread here is the following conditions:

  • The app you want to make migrations for has an FK to an unmigrated app (app w/o migrations)
  • At least one app in your project has existing migrations.

Basically, MigrationAutodetector is excluding from_state.real_apps, but real_apps is only populated when you have existing migrations (due to how the MigrationGraph instantiates from_state). I can't see any reason why autodetector should exclude real_apps, since it depends on these in order to determine external dependencies.

I've attached a .diff that fixes the issue and passes all tests, but I've no idea if it's the right call.

comment:10 Changed 4 years ago by anonymous

Same thing happens while running makemigrations for swappable user model: KeyError: 'auth'. I have another app with existing migrations in the same project, but it is completely unrelated to custom user (no FKs). If I disable this second app, makemigration succeeds.

comment:11 Changed 4 years ago by Andrew Godwin

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top