Opened 9 years ago

Closed 9 years ago

Last modified 11 months ago

#23694 closed Bug (wontfix)

Swappable Dependency should not use "__first__"

Reported by: Richard Eames Owned by: nobody
Component: Migrations Version: 1.7
Severity: Normal Keywords:
Cc: github@…, Andrew Godwin, Shai Berger Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Along the same vein as the other bugs I've recently reported about changing a model's db_table option, it seems that changing it for AUTH_USER_MODEL breaks any henceforth migrations that will depend on it.

To reproduce, try something a long the following:

  1. Start a new project and app with a customer AUTH_USER_MODEL, say Employee
  2. Set the db_table of Employee to something, 'store_employee'
  3. Create another model in another app

djbug

djbug/settings.py -> AUTH_USER_MODEL = 'app.Employee'
/app1/models.py

# /app1/models.py
class Employee(models.Model):
    class Meta:
        db_table = 'store_employee'

# /app2/models.py
class Store(models.Model):
    pass
  1. Create and run migrations.
  2. Change the db_table of Employee to the default, 'app_employee'.
  3. Create an run the migrations with something like migrations.AlterModelTable('Employee', 'accounts_employee')
  4. Add a many to many pointing to Employee
    # /app2/models.py
    class Store(models.Model):
        employees = models.ManyTomanyField(settings.AUTH_USER_MODEL)
    
  5. Create a new migration.

The migration generated will include the migrations.swappable_dependency(settings.AUTH_USER_MODEL) dependency, which expands to ('app1.Employee', '__first__'). The problem with this, is that at __first__, the db_table option is pointing to a now non-existent table.

I propose that migrations.swappable_dependency instead use either __latest__, or find out what the current migration is for the application containing the AUTH_USER_MODEL.

Change History (4)

comment:1 by Tim Graham, 9 years ago

Cc: Andrew Godwin added

I know there was some things that used __latest__, but were later switched to use __first__ due to issues. Adding Andrew to comment.

comment:2 by Andrew Godwin, 9 years ago

Resolution: wontfix
Status: newclosed

Unfortunately, if it's set to __latest__ you get way more circular dependencies incredibly easily and other nasty categories of bugs - it's actually impossible to have a fixed dependency choice for custom user models that works, as changing the setting can dynamically change the model set without any detectable change by Django when it next runs.

This limitation (needing to have custom user models in the first migration) is documented: https://docs.djangoproject.com/en/dev/topics/auth/customizing/#substituting-a-custom-user-model

While I'd love to know a way round it, __latest__ won't work, and "finding the migration containing the user model" is possible but nasty to implement. By enforcing this way we at least guarantee a lot less circular dependency errors later by giving you an error up front if it's wrong.

comment:3 by Mariusz Felisiak, 11 months ago

#34623 was closed as a duplicate.

comment:4 by Shai Berger, 11 months ago

Cc: Shai Berger added

Quoting myself from the duplicate ticket:

Another solution may be to load the app's migrations and look for a suitable CreateModel operation. That is not ideal, because models can be created by other operations (obviously the built-in RenameModel, but even an AddField with a M2M, not to mention 3rd-party operations).

A different approach would be just to check -- when a swappable dependency is in place, after running the initial migration of the dependency app, verify that the requested model exists, and if it doesn't, error out. That may later be extended, by allowing a specific migration to be marked (explicitly, in its code) as providing a specific model, but that extension may well be a YAGNI.

The above may justify reopening the ticket, as I believe it provides a partial answer to Andrew's comment:2 asking for a way around the limitations.

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