[Django] #31837: Possible manager migration bug
Django
------------------------------------------+------------------------
Reporter: Gordon Wrigley | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 3.0
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------+------------------------
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)
--
Ticket URL: <https://code.djangoproject.com/ticket/31837>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Gordon Wrigley | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 3.0
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Gordon Wrigley:
Old description:
> 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)
New description:
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.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/31837#comment:1>
Django
Reporter: Gordon Wrigley | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 3.0
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Gordon Wrigley):
Behaviour is the same on 3.1rc1
--
Ticket URL: <https://code.djangoproject.com/ticket/31837#comment:2>
Django
Reporter: Gordon Wrigley | Owner: nobody
Component: Migrations | Version: 3.0
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* status: new => closed
* resolution: => invalid
* component: Uncategorized => Migrations
Comment:
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 [https://docs.djangoproject.com/en/3.0/topics/migrations
/#model-managers "Model managers" docs].
Closing per TicketClosingReasons/UseSupportChannels.
--
Ticket URL: <https://code.djangoproject.com/ticket/31837#comment:3>
Django
Reporter: Gordon Wrigley | Owner: nobody
Component: Migrations | Version: 3.0
Severity: Normal | Resolution: invalid
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/31837#comment:4>
Django
Reporter: Gordon Wrigley | Owner: nobody
Component: Migrations | Version: 3.0
Severity: Normal | Resolution: invalid
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by felixxm):
> 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
[https://docs.djangoproject.com/en/3.0/topics/db/managers/#default-
managers 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/31837#comment:5>
Django
Reporter: Gordon Wrigley | Owner: nobody
Component: Migrations | Version: 3.0
Severity: Normal | Resolution: invalid
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/31837#comment:6>