Possible manager migration bug

[Gordon Wrigley]

If I understand managers and migrations correctly then the change in this commit ​https://github.com/tolomea/django-tutorial/commit/e5df712ef10be91cd2e5d28c9df48e33254a2b10 produces a bad migration.

Specifically I expect the migration to include both managers but it omits the "objects" manager.

This causes a problem in the followup data migration ​https://github.com/tolomea/django-tutorial/commit/90825038c55913d74a23b1c5c2ba35268d0a1367 where Question.objects isn't available leading to AttributeError: type object 'Question' has no attribute 'objects'

I have observed this in 3.0.8 and 2.2.14 (where I first ran into it)

Also I observed (on 2.2.14 didn't check this on 3.0.8) that adding objects to the concrete model (or an additional abstract model in between) fixes the problem.

[Gordon Wrigley]

Behaviour is the same on 3.1rc1

[Mariusz Felisiak]

Thanks for this report, IMO this is not a bug. You should define use_in_migrations = True on the custom manager class for objects (see #26643) and ​"Model managers" docs.

Closing per TicketClosingReasons/UseSupportChannels.

[Gordon Wrigley]

Well:
1: Both managers are declared in exactly the same way so why is one in and one out?
2: They are both actually instances of the Django base manager so they have the same setting for use_in_migrations (I presume True)
3: If you swap the order of the manager declarations around in the source code it no longer generates any migration
4: If you leave the order as is in the linked commit but additionally redeclare the objects manager in the concrete model classes it again decides no migration is needed

If this example seems a lil simplistic that's because I was trying to get a minimal reproduction, here's one that is structurally equivalent to what I actually have:
​https://github.com/tolomea/django-tutorial/commit/f247b6ddc5b2b689ec7ec0ede7cb0bc28c99db0c
Where everything prefixed lib is in a 3rd party library.

In there you can see that overriding objects on the concrete model causes both to be added to the migration.
Switching the order of the managers around also leads to a migration adding both managers to both concrete models.

[Mariusz Felisiak]

2: They are both actually instances of the Django base manager so they have the same setting for use_in_migrations (I presume True)

No, by default the use_in_migrations attribute is False.

Let's start from the beginning. Django includes in migrations a base manager, a default manager and managers with use_in_migrations set to True. The order matters because the first Manager Django encounters (in the order in which they’re defined in the model) has a special status, it's interpreted as the “default” Manager (see ​docs). Moreover to prevent unnecessary migrations we added in #26643 check to omit migrations if only one manager meet requirements and it's called objects.

3: If you swap the order of the manager declarations around in the source code it no longer generates any migration.

Yes because the first (and the default) manager is called objects, and it's the only manager that meets requirements to be included in migrations. In such cases we omit it to prevent unnecessary migrations (see #26643). The second manager doesn't have use_in_migrations set to True so it's ignored.

4: If you leave the order as is in the linked commit but additionally redeclare the objects manager in the concrete model classes it again decides no migration is needed

Yes because that changes the default manager to objects.

If this example seems a lil simplistic that's because I was trying to get a minimal reproduction, here's one that is structurally equivalent to what I actually have:
​https://github.com/tolomea/django-tutorial/commit/f247b6ddc5b2b689ec7ec0ede7cb0bc28c99db0c
Where everything prefixed lib is in a 3rd party library.

In there you can see that overriding objects on the concrete model causes both to be added to the migration.
Switching the order of the managers around also leads to a migration adding both managers to both concrete models.

lib_manager is added because it's declared as a base manager, objects is added because it's a default manager, so we have two managers which meets requirements. That's why both are included in migrations.

[Gordon Wrigley]

Thank you for walking me through that. I had missed the bit about the default manager.
Maybe we should change the fake models to fail with an explanation when the objects manager isn't included migrations and the user tries to access it.