On django multi tenancy

[Riccardo Magliocchetti]

Hi everyone,

i'd like to start a discussion about multi tenancy on this list. I've
been asked to move the discussion here by Aymeric in this ticket:

https://code.djangoproject.com/ticket/17802

First we already have an initial patch sitting in this ticket:

https://code.djangoproject.com/ticket/15089

patch here:

https://code.djangoproject.com/attachment/ticket/15089/15089-missing-site_id.diff

The patch basically change the SiteManager to return a Site based on the
request instead of the hardcoded settings.SITE_ID. Which while useful
for my usage is also making ticket 17802 obsolete since in the mean time
Sitemap has been switched to using get_current_site.

Another missing bit is having the request available everywhere, one
solution is to store in thread local storage that could be done easily
in a middleware without touching django core.

Another interesting thing (to me at least) would be being able to use a
different model for the Site so one can stores some site specific
configuration there (keeping in mind that in the patch sites return but
get_current_site are cached).

That could be obtained through this patch:

https://github.com/apollo13/django/compare/ticket15089

or maybe like what is done for the custom User model.

There's some more features for full multi tenancy listed here
https://code.djangoproject.com/ticket/15089#comment:36

but the above will suffice for my usage so i will not dig into it.

So to summarize with just a couple of patches to the site framework and
a dozen lines middleware one could build a basic django multi tenancy site.

How can I help to have this in 1.7?

thanks in advance,
riccardo

[Russell Keith-Magee]

On Thu, Apr 3, 2014 at 2:20 AM, Riccardo Magliocchetti <riccardo.ma...@gmail.com> wrote:

Hi everyone,

i'd like to start a discussion about multi tenancy on this list. I've been asked to move the discussion here by Aymeric in this ticket:

https://code.djangoproject.com/ticket/17802

First we already have an initial patch sitting in this ticket:

https://code.djangoproject.com/ticket/15089

patch here:

https://code.djangoproject.com/attachment/ticket/15089/15089-missing-site_id.diff

The patch basically change the SiteManager to return a Site based on the request instead of the hardcoded settings.SITE_ID. Which while useful for my usage is also making ticket 17802 obsolete since in the mean time Sitemap has been switched to using get_current_site.

Another missing bit is having the request available everywhere, one solution is to store in thread local storage that could be done easily in a middleware without touching django core.

-1. Not. Going. To. Happen.

Search the archives of Django Developers if you want to know why. This has been discussed to death.

Short version - it doesn't matter what pretty name you put on it, a thread local is a global variable. We teach first year programmers not to use globals, so why would we introduce them to Django as a core framework idea?

 

Another interesting thing (to me at least) would be being able to use a different model for the Site so one can stores some site specific configuration there (keeping in mind that in the patch sites return but get_current_site are cached).

That could be obtained through this patch:

https://github.com/apollo13/django/compare/ticket15089

or maybe like what is done for the custom User model.

What's the use case here? What additional data do you want to track?

 

There's some more features for full multi tenancy listed here
https://code.djangoproject.com/ticket/15089#comment:36

but the above will suffice for my usage so i will not dig into it.

Broadly speaking the approach you've described in #17802 makes sense to me - that is, find somewhere where the request is needed, and make sure it's available. I haven't dug in to the full consequences of introducing the request where you've put it, but the broad strokes look right. 

Unfortunately, we're not going to rush into something here - once we add or change an API, we need to support it, and we are committing to not changing it - so before we make any changes, we need to make sure we're not backing ourselves into a corner. That means solving the general problem (or at least having an idea how we would address the general problem), not just one small part of the problem.

The other approach that you haven't made mention of - Use a different sites app altogether. Django's sites module isn't well designed for true multi tenancy of the type that you're describing. However, it's also a contrib app -- so it's entirely optional, and also replaceable. If you have a specific set of requirements for multi tenancy, then write your own third-party app to implement those changes, and use it. 

 

So to summarize with just a couple of patches to the site framework and a dozen lines middleware one could build a basic django multi tenancy site.

How can I help to have this in 1.7?

Also - Not Going to Happen. 1.7 beta has been released, so the window for new features is closed. The best you can hope for at this point is for inclusion in Django 1.8.

Yours

Russ Magee %-)

[patric...@gmail.com]

Short version - it doesn't matter what pretty name you put on it, a thread local is a global variable. We teach first year programmers not to use globals, so why would we introduce them to Django as a core framework idea?

Aren't database connections thread local in Django?

[Michael Manfre]

On Wed, Apr 2, 2014 at 7:42 PM, <patric...@gmail.com> wrote:

Short version - it doesn't matter what pretty name you put on it, a thread local is a global variable. We teach first year programmers not to use globals, so why would we introduce them to Django as a core framework idea?

Aren't database connections thread local in Django?

Yes, but that is due to PEP 249 [1] allowing database drivers to not have thread safe connections. Django didn't have much of a choice about making connections thread local. Well, there was the choice to not follow PEP 249, but that would have been a foolish decision.

Regards,

Michael Manfre

[1] http://legacy.python.org/dev/peps/pep-0249/#threadsafety

[Russell Keith-Magee]

On Thu, Apr 3, 2014 at 7:42 AM, <patric...@gmail.com> wrote:

Short version - it doesn't matter what pretty name you put on it, a thread local is a global variable. We teach first year programmers not to use globals, so why would we introduce them to Django as a core framework idea?

Aren't database connections thread local in Django?

I'm tempted to just say "Please, go read the old threads", because I'm not going to be saying anything new here, and I'm running the risk of giving oxygen to a debate that I have exactly zero interest in revisiting.

Yes, Django does use threadlocals in a limited number of places. 

However:

 1) They're used as a mechanism of last resort

 2) They are exposed through APIs that hide this fact as much as possible.

 3) They're consistently sources of development pain for the core team, for all the reasons that global variables cause problems generally.

Putting request into threadlocals would mean putting active user state into a thread local, and encouraging users to interact with that user state as a global variable that "just exists".

It also ignores the fact that in many situations - for example, worker scripts - there is no such thing as a "current request". 

If I had a magic wand and could eliminate all the uses of threadlocals in our current codebase, I would. However, it's not that simple, for a combination of backwards compatibility reasons, and practical reasons. I'm yet to see an argument for thread local requests that doesn't boil down to "I don't want to pass a request object around as an argument."

For the record, I'll reiterate my *Strong* -1 against threadlocal requests (if it wasn't already obvious).

Yours,

Russ Magee %-)

[Riccardo Magliocchetti]

Hello,

Il 03/04/2014 00:15, Russell Keith-Magee ha scritto:
>
> On Thu, Apr 3, 2014 at 2:20 AM, Riccardo Magliocchetti
> <riccardo.ma...@gmail.com

> <mailto:riccardo.ma...@gmail.com>> wrote:
>
> Hi everyone,
>
> i'd like to start a discussion about multi tenancy on this list.
> I've been asked to move the discussion here by Aymeric in this ticket:
>

> https://code.djangoproject.__com/ticket/17802

> <https://code.djangoproject.com/ticket/17802>
>
> First we already have an initial patch sitting in this ticket:
>

> https://code.djangoproject.__com/ticket/15089
> <https://code.djangoproject.com/ticket/15089>
>
> patch here:
>
> https://code.djangoproject.__com/attachment/ticket/15089/__15089-missing-site_id.diff

> <https://code.djangoproject.com/attachment/ticket/15089/15089-missing-site_id.diff>
>
> The patch basically change the SiteManager to return a Site based on
> the request instead of the hardcoded settings.SITE_ID. Which while
> useful for my usage is also making ticket 17802 obsolete since in
> the mean time Sitemap has been switched to using get_current_site.
>
> Another missing bit is having the request available everywhere, one
> solution is to store in thread local storage that could be done
> easily in a middleware without touching django core.
>
>
> -1. Not. Going. To. Happen.
>
> Search the archives of Django Developers if you want to know why. This
> has been discussed to death.
>
> Short version - it doesn't matter what pretty name you put on it, a
> thread local is a global variable. We teach first year programmers not
> to use globals, so why would we introduce them to Django as a core
> framework idea?

I expected that :) i meant that if one wants that this can be done
without touching django at all.

> Another interesting thing (to me at least) would be being able to
> use a different model for the Site so one can stores some site
> specific configuration there (keeping in mind that in the patch
> sites return but get_current_site are cached).
>
> That could be obtained through this patch:
>

> https://github.com/apollo13/__django/compare/ticket15089

> <https://github.com/apollo13/django/compare/ticket15089>
>
> or maybe like what is done for the custom User model.
>
>
> What's the use case here? What additional data do you want to track?

For an ecommerce site "prices are without vat", "you need a vat id to
register", something like that. Again this would nice but since one can
use a separate model that's not a showstopper.

>
> There's some more features for full multi tenancy listed here

> https://code.djangoproject.__com/ticket/15089#comment:36

> <https://code.djangoproject.com/ticket/15089#comment:36>
>
> but the above will suffice for my usage so i will not dig into it.
>
>
> Broadly speaking the approach you've described in #17802 makes sense to
> me - that is, find somewhere where the request is needed, and make sure
> it's available. I haven't dug in to the full consequences of introducing
> the request where you've put it, but the broad strokes look right.

I was on the "add the request everywhere camp" until i've seen the
15089-missing-site_id.diff patch. Lot of places have been converted to
get_current_site so with the patch applied a site aware site would come
for free. Let me mention that this does not change default behaviour.

> Unfortunately, we're not going to rush into something here - once we add
> or change an API, we need to support it, and we are committing to not
> changing it - so before we make any changes, we need to make sure we're
> not backing ourselves into a corner. That means solving the general
> problem (or at least having an idea how we would address the general
> problem), not just one small part of the problem.
>
> The other approach that you haven't made mention of - Use a different
> sites app altogether. Django's sites module isn't well designed for true
> multi tenancy of the type that you're describing. However, it's also a
> contrib app -- so it's entirely optional, and also replaceable. If you
> have a specific set of requirements for multi tenancy, then write your
> own third-party app to implement those changes, and use it.

It's entirely optional but lot of other contrib code knows about it
(grep get_current_site). So using something else would be quite painful
if you want to reuse other apps.

> Also - Not Going to Happen. 1.7 beta has been released, so the window
> for new features is closed. The best you can hope for at this point is
> for inclusion in Django 1.8.

Yeah, thought about that after hitting enter :)

thanks,
riccardo

[Riccardo Magliocchetti]

Hello,

Il 03/04/2014 09:46, Riccardo Magliocchetti ha scritto:
> Hello,
>
> Il 03/04/2014 00:15, Russell Keith-Magee ha scritto:
>>
>> On Thu, Apr 3, 2014 at 2:20 AM, Riccardo Magliocchetti
>> <riccardo.ma...@gmail.com
>> <mailto:riccardo.ma...@gmail.com>> wrote:
>>
>> First we already have an initial patch sitting in this ticket:
>>
>> https://code.djangoproject.__com/ticket/15089
>> <https://code.djangoproject.com/ticket/15089>
>>
>> patch here:
>>
>>
>> https://code.djangoproject.__com/attachment/ticket/15089/__15089-missing-site_id.diff

I updated the patch to work againt latest git master, test passes [1]
and doc builds fine.

https://code.djangoproject.com/attachment/ticket/15089/0001-By-using-the-request-to-do-the-Site-matching.patch

thanks,
riccardo

[1] Actually i have this "ERROR: test_righthand_power
(expressions_regress.tests.ExpressionOperatorTests)"
there with "IntegrityError: NOT NULL constraint failed:
expressions_regress_number.the_integer" but it's completely unrelated
to sites framework, traceback here http://pastebin.com/MSmu8mi9

[Yo-Yo Ma]

It's so easy to build an Account or Tenant or Site model of your own, do a little leg work, and you've got a multitenant app with all the custom functionality you need. Getting caught up in trying to use contrib apps and hacking things together turned out to be more work once you pass the basics.

[Eduardo Rivas]

Mezzanine, a Django based CMS, has true multi tenancy. I've used it and works great. However, it's clear it's not something that should be in Django's core, for the reasons other have stated. I'm just bringing it up in case OP is still looking for something like this for his projects; not trying to restart the debate.

http://mezzanine.jupo.org/docs/multi-tenancy.html
https://github.com/stephenmcd/mezzanine/blob/master/mezzanine/utils/sites.py#L12

[Riccardo Magliocchetti]

Hi Eduardo,

[adding Stephen McDonald to CC]

Thanks for the info, was not aware of that. The functionality looks the
same as the patch below [1] attached to #15089 except they save the
site_id for the request in an external cache. It looks like if the patch
goes in they could leverage get_current_site(request) with empty
settings.SITE_ID just fine.

Let me say again the what i'm pushing to have in django is to have
django.contrib.sites return a Site based on the request instead of the
one hardcoded in settings.SITE_ID.

thanks,
riccardo

[1]
https://code.djangoproject.com/attachment/ticket/15089/0001-Make-settings.SITE_ID-optional-for-django.contrib.si.patch

[Robert Grant]

Does Ikari do what you want? I'm not saying it should or shouldn't be in Django contrib itself, but at least worth taking a look and seeing if that has any useful ideas.

[Riccardo Magliocchetti]

Hello,

Il 06/08/2014 16:34, Robert Grant ha scritto:
> Does Ikari <https://github.com/airtonix/django-ikari> do what you want?

> I'm not saying it should or shouldn't be in Django contrib itself, but
> at least worth taking a look and seeing if that has any useful ideas.

Thanks for sharing, the patch discussed in thread could be a considered
a building block for something like ikari.

> On Wednesday, 30 April 2014 09:37:13 UTC+2, riccardo.magliocchetti wrote:
>
> Hi Eduardo,
>
> [adding Stephen McDonald to CC]
>
> Il 29/04/2014 21:43, Eduardo Rivas ha scritto:
> > Mezzanine, a Django based CMS, has true multi tenancy. I've used
> it and
> > works great. However, it's clear it's not something that should
> be in
> > Django's core, for the reasons other have stated. I'm just
> bringing it
> > up in case OP is still looking for something like this for his
> projects;
> > not trying to restart the debate.
> >
> > http://mezzanine.jupo.org/docs/multi-tenancy.html

> <http://mezzanine.jupo.org/docs/multi-tenancy.html>
> >
> https://github.com/stephenmcd/mezzanine/blob/master/mezzanine/utils/sites.py#L12

> <https://github.com/stephenmcd/mezzanine/blob/master/mezzanine/utils/sites.py#L12>
>
>
> Thanks for the info, was not aware of that. The functionality looks the
> same as the patch below [1] attached to #15089 except they save the
> site_id for the request in an external cache. It looks like if the
> patch
> goes in they could leverage get_current_site(request) with empty
> settings.SITE_ID just fine.
>
> Let me say again the what i'm pushing to have in django is to have
> django.contrib.sites return a Site based on the request instead of the
> one hardcoded in settings.SITE_ID.
>
> thanks,
> riccardo
>
> [1]
> https://code.djangoproject.com/attachment/ticket/15089/0001-Make-settings.SITE_ID-optional-for-django.contrib.si.patch

> <https://code.djangoproject.com/attachment/ticket/15089/0001-Make-settings.SITE_ID-optional-for-django.contrib.si.patch>