On 21/11/2010, at 12:41 PM, Carl Meyer <carl.j.me...@gmail.com> wrote:
> Hi all, > > I've recently been exploring simple multitenancy options in Django > using contrib.sites, and have some thoughts on how core could make it > easier. ... > A few options for how such a hook could be implemented: ... > 1a) Same as above, but reuse SITE_ID; i.e. it can either be an > integer, or a string path to a get_current_site function. It ends up a > bit misnamed. This approach - or a variant of it - seems like the best idea to me. Rather than providing a custom hook for get_current_site(), I'd argue that we should make SITE_ID a callable that is used internally by Site.objects.get_current(). My reason for this is to avoid confusion over Site.objects.get_current(). At present, it only returns the right answer if the Sites model is installed. However, it is a method on a model, so this makes a certain amount of sense. If the extension point is get_current_site(), we could end up in a situation where the method exists, but may give you the wrong answer if you have a custom get_current_site() method. So - I'd propose the following: * add an optional request argument to Site.objects.get_current() * allow SITE_ID to be a callable that accepts the request as an argument * raise an error if SITE_ID is a callable, but Site.objects.get_current() is called without a request object being provided. * get_current_site() remains the preferred public interface for getting the current site. Essentially, this just means changing line 18 of sites/models.py [1] to check for a callable, and invoke that callable if required. [1] http://code.djangoproject.com/browser/django/trunk/django/contrib/sites/models.py?rev=13980#L18 The question this leaves is whether a SITE_ID callable returns an ID, or a full site object. I agree that returning an object from a method called _ID is less than ideal, but that's a relatively small wart given the corner that we're in, IMHO. The alternative, of course, is to completely deprecate Site.objects.get_current(), and use a new setting to configure get_current_site(). To be honest, this really wouldn't bother me either. This is the sort of area where a setting is the right solution, and deprecating Site.objects.get_current() avoids the confusion over the "right" way to get the current site. Yours, Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
