Carl and I had a long discussion in IRC about all the issues he raises above. I am still digesting that conversation, but there are some things that already spring to mind.
1. I can see the merits of defining a SITE_BACKEND api that would allow users to choose from different site-retrieval implementations that sites framework would contain. I wonder, would it make sense to make SITE_BACKEND the central aspect of the framework? SITE_BACKEND could be the required setting (instead of SITE_ID), and a "get_orm_site_by_id" backend, a "get_orm_site_by_request" backend, and a "get_request_site" backend could all be defined. That way, the documentation could say: "Always define a SITE_BACKEND, here are the different implementations to choose from." It would provide a good opportunity to discuss the different architectures in parallel, and would require users to explicitly choose from among mutually-explicit specific strategies, with only one setting. As it stands now with both of our proposals, documenting how all this works requires describing a very intricate and confusing conditional tree. Making the SITE_BACKEND control everything would simplify documentation significantly. 2. One difference between our proposals is that I propose that the framework's API define a *single*, unitary facade function--"get_current_site()", which takes a an optional "require_site_object" as a parameter--while Carl proposes that the framework define two facade functions--"get_current_site()", and the "Site.objects.get_current()" manager method. I see these two approaches as logically equivalent. Both proposals recognize the need to differentiate between the case where any generic site object may be returned (RequestSite, django.contrib.sites.models.Site, or some user-defined object), and the case where only django.contrib.sites.models.Site may be returned. Carl's proposal is more consistent with the Django practice of putting orm-object-retrieval logic inside managers, and avoids having to deprecate Site.objects.get_current(). My proposal more closely follows the facade design pattern, and I believe simplifies the documentation and thus simplifies things for users. I would favor my proposal, obviously, but I can understand the merits of Carl's. Also, Carl's proposal has the benefit of being what is already checked in. 3. In spite of the fact that having two facade functions does abrogate the need to define a "require_site_object" parameter in get_current_site, I think an argument can be made to include the "require_site_object" parameter in the API for SITE_BACKEND. Carl proposes verifying the object returned by SITE_BACKEND before returning from the manager method, to make sure it is in fact a Site object. If a user-defined SITE_BACKEND is coded to retrieve a custom object from the db, then an unnecessary database call can be prevented if "require_site_object" is passed in and used to return None at the top of the function. 4. I think it is important to keep in mind what the original motivation was behind #15089 and the post that started this thread: to remedy architectural inadequacies that forced the Satchmo devs, in their implementation of django-threaded-multihost, to monkey-patch SiteManager. Although using thread-locals to store the current request is something that Django frowns upon, it is the only way to achieve certain multitenancy behaviors, and seems to be required by a decent number of users (between them, django-threaded-multihost and django-multihost have 37 followers and 4 forks, and I am sure there are a number of other implementations out there). If SITE_BACKEND is called only when a request is supplied as an optional argument to Site.objects.get_current(), then the thread locals strategy employed by django-threaded-multihost et al. will still require monkey-patching SiteManager. If SITE_BACKEND is being called inside Site.objects.get_current(), it *should* be called *regardless* of whether a request is passed into it. If a particular SITE_BACKEND requires that a request be passed in for it to do properly retrieve a site, then that backend should raise an exception; it should not be that such a SITE_BACKEND is simply ignored. This is distinct from Carl's proposal, but I think not a significant deviation from it. Regards, Eduardo -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@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.
