contrib.sites and multitenancy
|Reported by:||legutierr||Owned by:||Florian Apolloner|
|Cc:||django@…, James Lecker Jr, Jari Pennanen, linovia, NicoEchaniz, dcwatson@…, hvdklauw@…, tgecho, fernando.sanchez@…, florian+django@…, krzysiumed@…, tom@…, net147, charette.s@…, w2lkm2n@…, simonkagwe, riccardo.magliocchetti@…, segfault||Triage Stage:||Accepted|
|Has patch:||yes||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
With reference to this thread:
Recent changes to django trunk made in  create a new site-retrieval function,
get_current_site(), which supplants
Site.objects.get_current(). However, in spite of the fact that
get_current_site() takes a request object as an argument, that request object is not being used to retrieve the current site; only the SITE_ID setting is used. Furthermore, there is a need to allow users to register their own site-retrieval functions, should they desire different functionality .
In this change, however, backwards compatibility must be maintained:
- if the SITE_ID setting is implemented, then that SITE_ID must be used to retrieve the site record in all cases where contrib.sites is installed.
- when contrib.sites is not installed, a RequestSite object should be returned
Specifically, the modification should contain the following elements:
- add a deprication warning to
- add a method
get_site_for_request()to Site.objects; this method will inspect the request, using
request.get_host(), and will then return the site object matching the request.
- add a method
get_site_for_id()to Site.objects, which method can be used by
get_current_site()in the case of SITE_ID being populated
- modify get_current_site() to handle four cases:
- If SITE_CALLBACK (a new setting) is defined, return the result of passing the request into that callable
- Else if contrib.sites is not installed, return a RequestSite
- Else if SITE_ID is defined, use
Site.objects.get_site_for_id()to get the site conforming to SITE_ID
- Otherwise, use
Site.objects.get_site_for_request()to retrieve the site that conforms to the request.
- add new tests to sites.tests
- modify the contrib.sites documentation to reflect the new approach and the new setting, SITE_CALLBACK
In addition, I would like to suggest two additional changes that are not documented in the above-referenced django-developers group thread:
- Move the
get_current_site()method out of the models.py file, and into
__init__.py. The thinking behind this is that referencing settings inside a models.py file has a bit of a code smell, and that
get_current_site(), now being the key control point of the library, should exist at a dependency level above the other modules.
- Add an additional keyword argument to get_current_site(), specifically
default_to_request_site, with default value
False. In the event that
default_to_request_siteis false, an error would be raised if Sites is not installed. The thinking behind this change is that third-party apps, the primary potential users of multitenancy implemented by contrib.sites, will most likely link their models to the Site object via a ForeignKey. As a result, RequestSite objects will produce errors if they are used, and additional logic will have to be implemented to special-case the handling of RequestSite objects in such a situation.
These changes, if pursued, would also require modifications in the other contrib apps that use RequestSite.
Change History (63)
comment:3 Changed 6 years ago by
|Triage Stage:||Unreviewed → Design decision needed|
comment:15 Changed 6 years ago by
|Patch needs improvement:||unset|
comment:17 Changed 6 years ago by
|Patch needs improvement:||set|
Changed 3 years ago by
Changed 2 years ago by