Opened 3 years ago

Closed 2 years ago

Last modified 2 years 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 3 years ago.
Fix circular dependency by building dependency graph in 2 stages

Download all attachments as: .zip

Change History (16)

comment:1 Changed 3 years ago by melinath

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

Changed 3 years ago by nliberg

Attachment: circular_dependency.patch added

Fix circular dependency by building dependency graph in 2 stages

comment:2 Changed 3 years 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 3 years ago by Ondrej Slinták

Cc: ondrowan@… added

comment:4 Changed 3 years ago by nliberg

Has patch: set

comment:5 Changed 3 years ago by melinath

Version: master1.7-beta-1

comment:6 Changed 3 years 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 3 years ago by Baptiste Mispelon

Triage Stage: UnreviewedAccepted

Triaging this per the previous comment

comment:8 Changed 3 years 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 2 years ago by chtimbo@…

Cc: chtimbo@… added

comment:10 Changed 2 years ago by Tim Graham

Patch needs improvement: set
Severity: NormalRelease blocker

comment:11 Changed 2 years ago by Andrew Godwin <andrew@…>

Resolution: fixed
Status: newclosed

In 5400b29ebfdcd8fb657bf42ddd6f81764b99f63a:

Fixed #22325: Ignore first dependencies to your own app

comment:12 Changed 2 years ago by Andrew Godwin <andrew@…>

In 1e8b1db050de72edc3c434e6e5d5dd55ae78ae2e:

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

comment:13 Changed 2 years ago by Andrew Godwin

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 2 years ago by Bas Peschier

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

comment:15 Changed 2 years ago by Jonas

Yep, works for me as well. Thanks!

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