Re: contrib.sites and multitenancy
On 21/11/2010, at 12:41 PM, Carl Meyer 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.
Re: contrib.sites and multitenancy
Hi Carl, Russell, I think any settings.py option will help us a lot, but doesn't the overall solution mean that one would still need to have the Site model installed even if we use our custom callable? I'd also like if someone could explain correct interfaces and if we're going to change them. Isn't Site/RequestSite the only possible interface right now? The more I thinking on it, the more I see we have the same problem like with the Users model: the only way to write reusable 3rd party app is to do item_site = ForeignKey(Site) which bounds us to the Site object. Also, will the request be passed to SITE_ID if it become a callable? If not, I'd like if we take get_current_site(request) instead of Site.get_current(), and deprecate the last one. On Sun, Nov 21, 2010 at 3:14 PM, Russell Keith-Magee wrote: > > > On 21/11/2010, at 12:41 PM, Carl Meyer 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. > -- Best regards, Yuri V. Baburov, ICQ# 99934676, Skype: yuri.baburov, MSN: bu...@live.com -- 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.
Re: Deprecating ADMIN_MEDIA_PREFIX
On 21/11/10 02:52, Carl Meyer wrote: > Hi all, > > The special-cased handling of contrib.admin static assets in Django > core is a long-time wart. Fortunately, the new static assets standard > introduced by contrib.staticfiles and the STATIC_URL and STATIC_ROOT > settings finally allows us to begin a migration path to remove this > wart. AdminMediaHandler is already on a deprecation path; the > remaining piece is the ADMIN_MEDIA_PREFIX setting. > > After discussion with Jannis on IRC, we have a plan for deprecating > ADMIN_MEDIA_PREFIX. The expectation is now that any apps' static > assets should be found at STATIC_URL/appname. The idea is to introduce > a check whether ADMIN_MEDIA_PREFIX is equal to STATIC_URL/admin, and > if it is not, raise a (pending) deprecation warning and alert the user > that in a future release of Django, the ADMIN_MEDIA_PREFIX setting > will be removed, and the admin media will be assumed to be served at > STATIC_URL/admin. > > Any objections to this plan before I put it into action? > > Carl > +1 from me; sounds like a good plan. The whole admin-media-special-case-thing has been a bit of a wart from the start. Andrew -- 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.
RFC #9964 - fix "missing" db commits by forcing managed transactions to close
Hi list, #9964 is about managed transactions not being committed under transaction middleware (or transaction.commit_on_success decorator) after the database was modified via raw SQL. The root cause of that is that, today, managed transactions only become "dirty" when there is clear evidence of database change -- that is, when a known-to-change-the-database operation (such as saving a model) is performed. If this isn't the case (as in the case of raw SQL), the user is required to call transaction.set_dirty() to get correct behavior. More generally, transactions which stay "clean" are not really closed by the transaction-management mechanism. If the transaction is part of a request, it will be implicitly rolled back at the end of the request when the connection is closed. If, under any circumstances, the connection stays open, so does the transaction; this is rare, but wrong, and a user will have no warning except for bugs creeping up. Yesterday I submitted a patch for this bug, which corrects this behavior by stipulating that every transaction where an operation was performed in managed mode should be considered dirty. In other words, there are no more "read-only transactions". Every (managed) transaction that is opened must be closed. This is a backward-incompatible change: most users, who either use automatic mode or some form of commit_on_success, should see no difference, but users working with commit_manually will be forced to close read-only transactions explicitly -- which they could get away with not doing before. I believe this leads to more correct code even in these cases, and all in all, it trades a silent gotcha in one situation for a clear exception in another: Under the current scheme, raw SQL execution can get rolled back silently if set_dirty() isn't called; while this is documented, it is a gotcha. Under my proposed fix, unclosed read-only transactions raise a TransactionManagementError. I think this is an overall improvement. But I also think more people would have an opinion about it, than those who follow http://code.djangoproject.com/ticket/9964. Your comments are welcome, Shai. -- 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.
