search

citusdata / django-multitenant

Python/Django support for distributed multi-tenant databases like Postgres+Citus
MIT License
708 stars 116 forks source link

Use asgiref when available instead of thread locals (#176) #184

Closed darwing1210 closed 9 months ago

darwing1210 commented 1 year ago

@microsoft-github-policy-service agree company="newsamp"

mjvankampen commented 12 months ago

@agedemenli this seems like a nice fix for #176

gurkanindibay commented 11 months ago

Hey @darwing1210 Here in your solution, the default implementation is asgiref.local We want to keep the backward compatibility. Could you add the support with a configuration parameter, which has the default value of threading.local? Additionally, we need a test to test both cases. To test the feature, you need to install asgiref package as well with the command pip install asgiref for asgiref test case

Thanks for your contribution @darwing1210

darwing1210 commented 11 months ago

@gurkanindibay I updated the PR with your notes, but I personally think that the setting is not necessary as asgiref is included as a Django Requirement since >= 3, please let me know any comments/improvements.

mjvankampen commented 9 months ago

@gurkanindibay any chance to get this merged?

gurkanindibay commented 9 months ago

@mjvankampen sorry for the delay. We have lots of things on our plate. We've recently added support for Django 4.1 and 4.2 in PR https://github.com/citusdata/django-multitenant/pull/197 I'm waiting it to be merged tomorrow. After merging this PR, we can test with 4.1 and 4.2 as well. Thanks for your contributions

gurkanindibay commented 9 months ago

@darwing1210 @mjvankampen There were conflicts about the dependencies. Since I could not push to the @darwing1210's fork repo, I needed to open a new PR. Could you check this PR? https://github.com/citusdata/django-multitenant/pull/198

gurkanindibay commented 9 months ago

Addressed with #198 Thanks @darwing1210 @mjvankampen for your contributions