Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check if better to use threading.local() #19

Open
filipeximenes opened this issue Sep 1, 2017 · 3 comments
Open

Check if better to use threading.local() #19

filipeximenes opened this issue Sep 1, 2017 · 3 comments

Comments

@filipeximenes
Copy link

Eg.: https://github.com/filipeximenes/multitenancy/blob/master/multitenancy/threadlocal.py

@techdragon
Copy link

While a useful and valid tool, it is generally considered something to be avoided unless necessary. As someone that just started using this library I'd prefer to see thread locals avoided as much as possible. Im not opposed to having helper functions/tools that use it, I just really don't want to see the entire library develop in a direction where it required using thread locals to work. - Just my 2 cents worth of thoughts as a user. 😃

@hugobessa
Copy link
Owner

@techdragon did you see we're already using threading on this package, right? https://github.com/hugobessa/django-shared-schema-tenants/blob/dev/shared_schema_tenants/middleware.py#L57

Don't you think with threading.local this would be more organized?

@techdragon
Copy link

I do agree it makes it more organised. However for the same reasons outlined in the rational section of PEP 567 I think thread local storage based solutions are less than ideal solutions where alternative ones exist. Why it concerns me with this library in particular is that I'm looking down the road a little way to a future where I have my Django View code running in an asynchronous environment like ASGI on top of Daphne or Pulsar, or the model code used by asyncio based task worker tools.

When I commented yesterday I was unaware of PEP 567 so I can now happily point out that because of PEP 567 Python >3.7 there is a better alternative (at least in Python 3.7 and above) specifically for the sort of asynchronous related issues that underly my concerns about relying on thread-local storage.

Case in point (which also provides a handy example) , the Decimal module now uses contextvars instead of threading. The bug report Migrate decimal to use PEP 567 context variables and the associated pull request to the python source code bpo-32630: Use contextvars in the decimal module seem to be good examples of how to do things more safely with respect to asynchronous code safety at least in >3.7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants