Opened 6 months ago

Closed 6 months ago

#23360 closed Uncategorized (invalid)

CircularDependencyError with AUTH_USER_MODEL set

Reported by: brian Owned by: nobody
Component: Migrations Version: 1.7-rc-3
Severity: Normal Keywords:
Cc: info@… Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Hello,

I am trying to get django-guardian working with Django 1.7, and I keep getting this error:

$ python setup.py test
...
django.db.migrations.graph.CircularDependencyError: [('admin', u'0001_initial'), ('testapp', u'0001_initial'), (u'admin', u'0001_initial')]

As far as I can tell, this is absolutely correct, there is a circular loop, only I don't know what to do about it. testapp/models.py contains:

    from django.contrib.auth.models import AbstractUser
    class CustomUser(AbstractUser, GuardianUserMixin):
        custom_id = models.AutoField(primary_key=True)

settings.py contains:

AUTH_USER_MODEL = "testapp.CustomUser"

This results in a migration that contains:

    dependencies = [
        ('auth', '0001_initial'),
        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
        ('admin', '0001_initial'),
    ]

If I understand correctly, testapp depends on auth because it uses AbstractUser which requires Group.

I am not sure I understand why the auth app depends on testapp.

Very experimental source with some other Django 1.7 fixes is available here:
https://github.com/brianmay/django-guardian/tree/patched

Thanks

Change History (10)

comment:1 Changed 6 months ago by Markush2010

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Thanks for the report. Just to make sure, you're really running 1.7-rc-3? I thought I fixed this problem in #23322.

comment:2 Changed 6 months ago by brian

Yes, this is 1.7rc3. Just regenerated the migrations from scratch, get the same problem.

Something rather complicated going on here, still trying to understand it. Think it might be somehow based on the fact there are two apps "guardian" and "testapp".

I tried to create a simplified test case with one app to reproduce the problem, but I couldn't reproduce the problem.

comment:3 Changed 6 months ago by brian

What does migrations.swappable_dependency do?

I assume it translates something like "testapp.CustomUser" into tuple identifying a migration, like ("testapp", "0001_initial").

Does it do anything else, reverse dependencies or anything?

comment:4 Changed 6 months ago by brian

Just noticed django/contrib/admin/migrations/0001_initial.py has:

class Migration(migrations.Migration):                                           
                                                                                   
      dependencies = [                                                             
          migrations.swappable_dependency(settings.AUTH_USER_MODEL),               
          ('contenttypes', '__first__'),                                           
      ]                                                                                                                                                               

Ok, starting to see why there is a circular dependency loop here. My simple application did not use django.contrib.admin. Ooops.

comment:5 Changed 6 months ago by brian

Sooo guardian.testapp.models.py has:

from django.contrib.admin.models import LogEntry

class LogEntryWithGroup(LogEntry):
    group = models.ForeignKey('auth.Group', null=True, blank=True)

This means testapp depends on admin, because it requires LogEntry.

django.contrib.admin.models has:

class LogEntry(models.Model):
    user = models.ForeignKey(settings.AUTH_USER_MODEL)

This means that admin depends on testapp, because settings.AUTH_USER_MODEL == "testapp.CustomUser". Or:

admin.LogEntry --> testapp.CustomUser
testapp.LogEntryWithGroup --> admin.LogEntry

Which clearly shows the circular dependency.

I was wondering what happens if you have two apps containing two models that depend on each other, think it isn't going to work. You could make the argument this was never really supported - in Django 1.6/South I have had problems with a similar situation with postgresql requiring the other table to be created before constraints are created, however it gave the illusion of working, at least for mysql.

In this particular case, wonder if might be possible to split the migration up into two. So that ('testapp', '0001_initial') creates CustomUser, admin depends on this, then ('testapp', '0002_LogEntryWithGroup') creates LogEntryWithGroup and depends on admin. Might have a play at this later.

comment:6 Changed 6 months ago by brian

Hello,

Splitting the migrate up does seem to have solved the problem. I deleted the migrations, deleted the LogEntryWithGroup model, created the migrations, added the LogEntryWithGroup model, created the migration. It seems to work.

I think there might be rare cases where it is required to tweak the automatically generated migrations like this.

Another case might be where you have two tables that have ForeignKeys to each other in different Apps. Yes, I have seen this. Might need to delete one of the foreign keys, create the migrations, add the foreign key back, and create a new migration. So it is clear what order things need to be created in.

Possibly this could be better documented.

In any case, if you want to close this ticket, I am happy with that.

Thanks

comment:7 Changed 6 months ago by Markush2010

  • Cc info@… added

comment:8 Changed 6 months ago by brian

Just another thought, splitting the migrations up might cause problems when upgrading from Django 1.6. Unless Django 1.7 is clever enough to realize it doesn't need to run the (for example) 0002_LogEntryWithGroup migration because the table already exists. I don't have a good test case for this.

comment:9 Changed 6 months ago by timgraham

The fact that you need to manually fake migrations when you have more than one initial migration is documented.

comment:10 Changed 6 months ago by andrewgodwin

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

Additionally, the fact that swappable models (e.g. custom user) cause circular dependencies is also documented: https://docs.djangoproject.com/en/dev/topics/auth/customizing/#substituting-a-custom-user-model

This is unavoidable, I'm afraid; you just have to manually fix the problem (it's the price you pay for swappable user functionality - uncertainty on the part of the dependency calculator). Closing as INVALID for that reason - I wish we could fix this, but I actually think it's technically impossible in the general case.

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