Opened 21 months ago

Closed 19 months ago

Last modified 19 months ago

#22325 closed Bug (fixed)

Defining a custom user model in an app and having relations between it and other models in the app causes circular dependencies

Reported by: melinath Owned by: nobody
Component: Migrations Version: 1.7-beta-1
Severity: Release blocker Keywords:
Cc: melinath, ondrowan@…, Jonas, chtimbo@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no


I *think* the issue is that swappable_dependency resolves to app_label.first, which doesn't make a lot of sense when the app with the migrations is itself being migrated. Removing that line seems to temporarily resolve the issue, but if my app were ever used with a *different* AUTH_USER_MODEL, it would then presumably break again. (Also, I still get a circular dependency in the next migration that defines a relation to AUTH_USER_MODEL.)

Here are the important bits from *one* migration:

class Migration(migrations.Migration):

    dependencies = [
        ('app_label', '0002_blah_blah'),

    operations = [
            # This is the AUTH_USER_MODEL
                u'verbose_name': u'person',
                u'verbose_name_plural': u'people',
                ('person', models.ForeignKey(to=settings.AUTH_USER_MODEL, to_field=u'id')),


Traceback (most recent call last):
  File "./", line 10, in <module>
  File ".../django/django/core/management/", line 427, in execute_from_command_line
  File ".../django/django/core/management/", line 419, in execute
  File ".../django/django/core/management/", line 288, in run_from_argv
    self.execute(*args, **options.__dict__)
  File ".../django/django/core/management/", line 337, in execute
    output = self.handle(*args, **options)
  File ".../django/django/core/management/commands/", line 103, in handle
    plan = executor.migration_plan(targets)
  File ".../django/django/db/migrations/", line 46, in migration_plan
    for migration in self.loader.graph.forwards_plan(target):
  File ".../django/django/db/migrations/", line 53, in forwards_plan
    return self.dfs(node, lambda x: self.dependencies.get(x, set()))
  File ".../django/django/db/migrations/", line 119, in dfs
    return _dfs(start, get_children, [])
  File ".../django/django/db/migrations/", line 111, in _dfs
    results = _dfs(n, get_children, path) + results
  File ".../django/django/db/migrations/", line 111, in _dfs
    results = _dfs(n, get_children, path) + results
  File ".../django/django/db/migrations/", line 111, in _dfs
    results = _dfs(n, get_children, path) + results
  File ".../django/django/db/migrations/", line 103, in _dfs
    raise CircularDependencyError(path[path.index(start):] + [start])
django.db.migrations.graph.CircularDependencyError: [('brambling', '0003_blah_blah'), ('brambling', '0003_blah_blah')]

Attachments (1)

circular_dependency.patch (2.1 KB) - added by nliberg 21 months ago.
Fix circular dependency by building dependency graph in 2 stages

Download all attachments as: .zip

Change History (16)

comment:1 Changed 21 months ago by melinath

  • Cc melinath added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Changed 21 months ago by nliberg

Fix circular dependency by building dependency graph in 2 stages

comment:2 Changed 21 months ago by nliberg

As far as I can see the error is in the build_graph method of the MigrationLoader class.
The self.graph.root_nodes(...) method seems to be invoked before the graph is fully built resulting in some extra nodes in the returned set that are root nodes in the current partial graph but are not root nodes in the final graph. Since the parent of the dependency relationship is drawn from this set of nodes one can end up with an incorrect loop from the node to itself in the graph. This results in a CircularDependencyError exception at a later stage.

The problem seem to be fixed by building the graph in two steps: first handling all normal nodes and then the "__first__" nodes (that get remapped to a root node). Someone with a deeper understanding of the relevant code will have to be the judge of whether this is a partial or complete fix. At least the patch above fixes the problem for me.

comment:3 Changed 21 months ago by ondrowan

  • Cc ondrowan@… added

comment:4 Changed 21 months ago by nliberg

  • Has patch set

comment:5 Changed 20 months ago by melinath

  • Version changed from master to 1.7-beta-1

comment:6 Changed 20 months ago by Jonas

  • Cc Jonas added

I can confirm this bug and that the patch resolves it. Will be happy to assist with further tests.

comment:7 Changed 20 months ago by bmispelon

  • Triage Stage changed from Unreviewed to Accepted

Triaging this per the previous comment

comment:8 Changed 20 months ago by Jonas

I submitted a pull request with a test here:

As mentioned in the pull request, the patch does not work for the test I added - while it does work for my actual code. This is the first test I wrote for Django itself so it might be due to some error on my part. Trying it for a while, I couldn't find it though. Would appreciate some help in getting this mergeable.

comment:9 Changed 19 months ago by chtimbo@…

  • Cc chtimbo@… added

comment:10 Changed 19 months ago by timo

  • Patch needs improvement set
  • Severity changed from Normal to Release blocker

comment:11 Changed 19 months ago by Andrew Godwin <andrew@…>

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

In 5400b29ebfdcd8fb657bf42ddd6f81764b99f63a:

Fixed #22325: Ignore first dependencies to your own app

comment:12 Changed 19 months ago by Andrew Godwin <andrew@…>

In 1e8b1db050de72edc3c434e6e5d5dd55ae78ae2e:

[1.7.x] Fixed #22325: Ignore first dependencies to your own app

comment:13 Changed 19 months ago by andrewgodwin

Jonas: I'm reasonably sure the patch I just pushed is the real fix, based on what I know of the problem, but would appreciate if you could make sure it works for your code.

(I forgot to add the test to that commit, it's in one just afterward. Thanks for helping with that)

comment:14 Changed 19 months ago by bpeschier

I can't speak for Jonas, but I had similar symptoms which as of this commit are nonexistent. Thanks!

comment:15 Changed 19 months ago by Jonas

Yep, works for me as well. Thanks!

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