Opened 10 years ago

Last modified 10 years ago

#23687 new Cleanup/optimization

Prevent `ContentType.objects.get_for_model` from creating objects for deferred, auto_created and swapped models

Reported by: ILYA Owned by: nobody
Component: contrib.contenttypes Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no
Pull Requests:How to create a pull request

Description

This ticket is extracted from #23444 for the sake of clarity. See initial discussion there.

Briefly:

ContentType.objects.get_for_model method returns appropriate ContentType object for the model if it exsits and creates one if not. But it doesn't check whether a model passed to it is deferred, auto_created or swapped.
Thus we get unnecessary ContentType objects in our DB that will be removed after next sync_db/migrate as django by default considers such CT objects as stale (and doesn't create them by itself anyway).

According to the ticket's flags, the next step(s) to move this issue forward are:

  • To provide a patch by sending a pull request. Claim the ticket when you start working so that someone else doesn't duplicate effort. Before sending a pull request, review your work against the patch review checklist. Check the "Has patch" flag on the ticket after sending a pull request and include a link to the pull request in the ticket comment when making that update. The usual format is: [https://github.com/django/django/pull/#### PR].

Change History (3)

comment:1 by Simon Charette, 10 years ago

Additional checks could added for the auto_created and swapped cases but the deferred one seems to be already correctly handled?

comment:2 by ILYA, 10 years ago

Yeah, seems like that.

So we have get_for_model and get_for_models mathod that need to be patched.
Checks for swapped and auto_created attrs are quite trivial but the question is what should we do in that case?
Should we raise an exception (e.g. ValueError as we get wrong model value) or simply return None to show that this model should not have CT object.
In the latter case I think we need also some docs notes for that.

Also we can consider whether we need some additional attrs for get_for_model so that it would create CT for swapped and auto_created models.

comment:3 by Tim Graham, 10 years ago

Triage Stage: UnreviewedAccepted

Raising an error seems more explicit.

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