Proof of concept: multi-tenant support using postgres schemas and django-tenants #123
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Pull Request Test Coverage Report for Build 520
💛 - Coveralls |
8 similar comments
Pull Request Test Coverage Report for Build 520
💛 - Coveralls |
Pull Request Test Coverage Report for Build 520
💛 - Coveralls |
Pull Request Test Coverage Report for Build 520
💛 - Coveralls |
Pull Request Test Coverage Report for Build 520
💛 - Coveralls |
Pull Request Test Coverage Report for Build 520
💛 - Coveralls |
Pull Request Test Coverage Report for Build 520
💛 - Coveralls |
Pull Request Test Coverage Report for Build 520
💛 - Coveralls |
Pull Request Test Coverage Report for Build 520
💛 - Coveralls |
Pull Request Test Coverage Report for Build 635
💛 - Coveralls |
Pull Request Test Coverage Report for Build 520
💛 - Coveralls |
9 similar comments
Pull Request Test Coverage Report for Build 520
💛 - Coveralls |
Pull Request Test Coverage Report for Build 520
💛 - Coveralls |
Pull Request Test Coverage Report for Build 520
💛 - Coveralls |
Pull Request Test Coverage Report for Build 520
💛 - Coveralls |
Pull Request Test Coverage Report for Build 520
💛 - Coveralls |
Pull Request Test Coverage Report for Build 520
💛 - Coveralls |
Pull Request Test Coverage Report for Build 520
💛 - Coveralls |
Pull Request Test Coverage Report for Build 520
💛 - Coveralls |
Pull Request Test Coverage Report for Build 520
💛 - Coveralls |
jcass77
force-pushed
the
enh/django_tenants
branch
from
f9187fc
to
1c379e7
Compare
|
I can't figure out why the tests are failing for python 2.7 - anybody have any ideas? |
|
@jcass77 Merge latest commits in master to check again. Lots of weird errors arose during testing that I patched up. |
|
That's an interesting error. It's a tox error regarding InvocationError. I've taken a look at this SO question and it seems like there might've been an import error. I haven't touched the code since the 2.2.0 release, so it's somewhere in your code base. @jcass77 |
|
I'm also coming to the realization that adding this could cause me to hassle on and understand Django-tenants. If I were to add this to the next release, it would be an overload of code to maintain, especially when Django 3 is trying to make a HUGE overhaul in how we handle databases (new release of psycopg3 in cooperation with the Django dev team in order to assist with accessing Postgres asynchronously while being thread safe). |
|
Thanks for taking a look at this @Andrew-Chen-Wang - I've merged in all of the latest changes from the 'master' branch. All of the tests are now green except for Django 1.11 running on Python 2.7. Since neither of those versions are officially supported anymore I'm not going to worry about it. I agree that this usecase might be too specific to merge into the official release of Django-cachalot. Would you mind keeping this pull request open, perhaps with a 'wontfix' label, so that others can find it more easily? |
|
@jcass77 I won't mind merging some of the engines into the package as I've decided to allow unobtrusive code into the repository. Stuff like the added settings.py supported backend can be merged in, and if you'd like to write a doc page along with how to integrate the code (somehow), I'd be happy to merge that in as well. It'd be best if you did this in a separate PR, and I'll leave this one open for reference, I suppose. But, for now, I still can't maintain this repository to support other packages' code integrations, so the tests for the new obtrusive code still can't fly. |
|
This is awesome =) |
# Conflicts: # cachalot/settings.py
|
This is great 👌 |
|
@jcass77 are you using this successfully in a production environment? |
Yes I was up to about the middle of 2021. I am no longer supporting that production environment though. |
@jcass77 What caching solutions are you currently using with Django tenants? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Not sure if this is too specific to be merged in django-cachalot, but posting it here in case it may be useful to anybody.
django-tenants makes use of postgres schemas to provide each tenant in a multi-tenant django deployment with their own database tables. This seems like a good usecase for django-cachalot as such deployments probably stand to benefit a lot from caching.
By default, django-tenants just adds the schema name to the cache keys so that each tenant can operate its own cache without risking unexpected collisions. This PR essentially just adds a new implementation of
make_keyso that all tables in thepublicschema can be shared between tenants:adds
django_tenants.postgresql_backendto the list of supported database backends, which is essentially just django-tenants' wrapper arounddjango.db.backends.postgresql_psycopg2for managing the schemas.introduces a new
get_multi_tenant_table_cache_keythat can be configured viasettings.CACHALOT_TABLE_KEYGENto generate the new cache keys - no effect on existing implementations unless explicitly activated.This PR could probably be easily adapted to also support django-tenant-schemas in a similar fashion.