[Django] #32833: ContentType.objects.get_for_models() in migrations does not works for multiple models
Django
multiple models
------------------------------------------------+------------------------
Reporter: Heraldo Lucena | Owner: nobody
Type: Bug | Status: new
Component: contrib.contenttypes | Version: 3.1
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 |
------------------------------------------------+------------------------
I am trying to use migrations to create default groups, I tried to run the
following procedure with `migrations.RunPython()`
{{{
def create_normal_users_group(apps, *args):
auth = SimpleNamespace(**apps.all_models['auth'])
myapp = SimpleNamespace(**apps.all_models['myapp'])
ContentType = apps.get_model('contenttypes', 'ContentType')
group = auth.group.objects.create(name='Normal Users')
contenttypes = ContentType.objects.get_for_models(myapp.user,
myapp.proxy) # there are more models...
# ...
}}}
but it raises `AttributeError`
{{{
File "/home/user/projects/myapp/.venv/lib/python3.8/site-
packages/django/contrib/contenttypes/models.py", line 89, in
get_for_models
opts_models = needed_opts.pop(ct.model_class()._meta, [])
AttributeError: 'ContentType' object has no attribute 'model_class'
}}}
it works when I pass a single model to
`ContentType.objects.get_for_models()`, but `get_for_models()` is supposed
to work with multiple models.
--
Ticket URL: <https://code.djangoproject.com/ticket/32833>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
multiple models
Reporter: Heraldo Lucena | Owner: nobody
Type: Bug | Status: new
contrib.contenttypes |
Severity: Normal | Resolution:
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 Heraldo Lucena:
Old description:
> I am trying to use migrations to create default groups, I tried to run
> the following procedure with `migrations.RunPython()`
>
> {{{
> def create_normal_users_group(apps, *args):
> auth = SimpleNamespace(**apps.all_models['auth'])
> myapp = SimpleNamespace(**apps.all_models['myapp'])
> ContentType = apps.get_model('contenttypes', 'ContentType')
> group = auth.group.objects.create(name='Normal Users')
> contenttypes = ContentType.objects.get_for_models(myapp.user,
> myapp.proxy) # there are more models...
> # ...
> }}}
> but it raises `AttributeError`
>
> {{{
> File "/home/user/projects/myapp/.venv/lib/python3.8/site-
> packages/django/contrib/contenttypes/models.py", line 89, in
> get_for_models
> opts_models = needed_opts.pop(ct.model_class()._meta, [])
> AttributeError: 'ContentType' object has no attribute 'model_class'
> }}}
> it works when I pass a single model to
> `ContentType.objects.get_for_models()`, but `get_for_models()` is
> supposed to work with multiple models.
New description:
I am trying to use migrations to create default groups, I tried to run the
following procedure with `migrations.RunPython()`
{{{
def create_normal_users_group(apps, *args):
auth = SimpleNamespace(**apps.all_models['auth'])
myapp = SimpleNamespace(**apps.all_models['myapp'])
ContentType = apps.get_model('contenttypes', 'ContentType')
group = auth.group.objects.create(name='Normal Users')
contenttypes = ContentType.objects.get_for_models(myapp.user,
myapp.proxy) # there are more models...
# ...
}}}
but it raises `AttributeError`
{{{
File "/home/user/projects/myapp/.venv/lib/python3.8/site-
packages/django/contrib/contenttypes/models.py", line 89, in
get_for_models
opts_models = needed_opts.pop(ct.model_class()._meta, [])
AttributeError: 'ContentType' object has no attribute 'model_class'
}}}
it works when I pass a single model to
`ContentType.objects.get_for_models()`, but `get_for_models()` is supposed
to work with multiple models.
EDIT:
Adding `model_class()` to `ContentType` makes it work
{{{
def create_normal_users_group(apps, *args):
auth = SimpleNamespace(**apps.all_models['auth'])
myapp = SimpleNamespace(**apps.all_models['myapp'])
ContentType = apps.get_model('contenttypes', 'ContentType')
# this makes it work =========================
def model_class(self):
return apps.get_model(self.app_label, self.model)
ContentType.model_class = model_class
# ========================================
group = auth.group.objects.create(name='Normal Users')
contenttypes = ContentType.objects.get_for_models(myapp.user,
myapp.proxy) # there are more models...
# ...
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/32833#comment:1>
Django
multiple models
Reporter: Heraldo Lucena | Owner: nobody
Type: Bug | Status: new
Component: contrib.contenttypes | Version: 3.1
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* stage: Unreviewed => Accepted
Comment:
Since `ContentTypeManager` is marked as `use_in_migrations = True` it
should be able to handle this case.
There's two possible solutions here:
1. Create an `AbstractContentType(models.Model)` abstract model class that
include most of the logic `ContentType` and have the latter subclass the
former. From there `CreateModel(bases)`
[https://github.com/django/django/blob/ed3af3ff4b3cfb72de598f1b39a1028ba3826efb/django/contrib/contenttypes/migrations/0001_initial.py#L25
should be adjusted to pass this base instead].
2. Adjust `ContentTypeManager.get_for_models` to avoid calling
`.model_class()` and inline its logic in the manager method instead.
Feels like 2. is the less disruptive approach and easier to test. Would
you be interested in writing a patch PR for it?
--
Ticket URL: <https://code.djangoproject.com/ticket/32833#comment:2>
Django
multiple models
Reporter: Heraldo Lucena | Owner: Heraldo
| Lucena
Type: Bug | Status: assigned
Component: | Version: 3.1
contrib.contenttypes |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Heraldo Lucena):
* owner: nobody => Heraldo Lucena
* status: new => assigned
Comment:
OK I will implement 2nd solution.
--
Ticket URL: <https://code.djangoproject.com/ticket/32833#comment:3>
Django
multiple models
Reporter: Heraldo Lucena | Owner: Heraldo
| Lucena
Type: Bug | Status: assigned
Component: | Version: 3.1
contrib.contenttypes |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Heraldo Lucena):
I tried to reproduce this problem but I could not, check my
[https://github.com/HMaker/django/commit/09d4ff8277464f4dc554d853fb22d47d42235e35
partial patch], it's based on 3.1.4 branch.
--
Ticket URL: <https://code.djangoproject.com/ticket/32833#comment:4>
Django
multiple models
Reporter: Heraldo Lucena | Owner: Heraldo
| Lucena
Type: Bug | Status: assigned
Component: | Version: 3.1
contrib.contenttypes |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Heraldo Lucena):
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/32833#comment:5>
Django
multiple models
Reporter: Heraldo Lucena | Owner: Heraldo
| Lucena
Type: Bug | Status: assigned
Component: | Version: 3.1
contrib.contenttypes |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Keryn Knight):
Replying to [comment:4 Heraldo Lucena]:
> I tried to reproduce this problem but I could not [...]
My guess would be that the ContentTypeManager instance already had a
partially filled (or empty) cache when you encountered the error, such
that `self._get_from_cache(modelinstance._meta)` worked (didn't throw a
`KeyError` looking in the `self._cache`).
- If you pass in 2 model classes `Group` and `User` and ''both'' are
already in its internal cache, then `needed_opts` is empty and
`needed_opts.pop(ct.model_class()._meta, [])` doesn't occur.
- If you pass the same 2 classes and only 1 or 0 of them are in the
`self._cache` already, you'd fill the missing items into `needed_opts` and
hit the line which has caused you problems.
--
Ticket URL: <https://code.djangoproject.com/ticket/32833#comment:6>
Django
multiple models
Reporter: Heraldo Lucena | Owner: Heraldo
| Lucena
Type: Bug | Status: assigned
Component: | Version: 3.1
contrib.contenttypes |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Heraldo Lucena):
I thought migrations would be automatically rolled back after each test
case, but that did not happen and last test case could not run properly
since all migrations were already applied.
--
Ticket URL: <https://code.djangoproject.com/ticket/32833#comment:7>
Django
multiple models
Reporter: Heraldo Lucena | Owner: Heraldo
| Lucena
Type: Bug | Status: assigned
Component: | Version: 3.1
contrib.contenttypes |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Heraldo Lucena):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/32833#comment:8>
Django
multiple models
Reporter: Heraldo Lucena | Owner: Heraldo
| Lucena
Type: Bug | Status: assigned
Component: | Version: 3.1
contrib.contenttypes |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/32833#comment:9>
Django
multiple models
Reporter: Heraldo Lucena | Owner: (none)
Type: Bug | Status: new
Component: contrib.contenttypes | Version: 3.1
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* owner: Heraldo Lucena => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/32833#comment:10>
Django
multiple models
Reporter: Heraldo Lucena | Owner: Sarah
| Boyce
Type: Bug | Status: assigned
Component: | Version: 3.1
contrib.contenttypes |
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Sarah Boyce):
* owner: (none) => Sarah Boyce
* needs_better_patch: 1 => 0
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/32833#comment:11>
Django
multiple models
Reporter: Heraldo Lucena | Owner: Sarah
| Boyce
Type: Bug | Status: assigned
Component: | Version: 3.1
contrib.contenttypes |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/32833#comment:12>
Django
multiple models
Reporter: Heraldo Lucena | Owner: Sarah
| Boyce
Component: | Version: 3.1
contrib.contenttypes |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by GitHub <noreply@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"84206607d6bfd61e7f7a88b51163ffd4153e3b5a" 84206607]:
{{{
#!CommitTicketReference repository=""
revision="84206607d6bfd61e7f7a88b51163ffd4153e3b5a"
Fixed #32833 -- Fixed ContentTypeManager.get_for_models() crash when using
in migrations.
Co-authored-by: Heraldo Lucena <2315551...@users.noreply.github.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32833#comment:13>