-
-
Notifications
You must be signed in to change notification settings - Fork 130
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
Added cache backend #22
base: master
Are you sure you want to change the base?
Conversation
Hi @arescope it looks like a nice addition. Did you base your work on that of Django's default cache backend? In order to merge it, it'll need some documentation and unit tests. Can you also provide those? |
Yes I am using redis as cache backend so it would be nice to have it merged :). Yes sure, I can do it during this week. Actually I was about to do it, but I prefer to talk with you with that before. Currently your tests are tied to the db backend, for instance you use Would you prefer to:
I am fine with all solutions, but let me know which one you feel more comfortable :) Best regards, Alberto |
Let's see....
So I think 3 is the most straight-forward solution and provides the best all-round test scenarios. Using a flag to set either cache backend, it would be possible to extend the test matrix of tox and travis. Thanks! |
Yup, I also thought that. I cannot give you an ETA for do it because now I am quite busy with my project, but will try as soon as possible. I want this merged :-). Thanks! |
What would be the added benefit of using this cache backend vs. the Django's cache backend? This project's selling point is that one can run a query over all sessions (in the database). However if a cache backend is used, no sessions can be run on them, right? So why not use Django's cache backend? |
The main class should probably be You cannot just use Django's session cache directly because the https://github.com/django/django/blob/master/django/contrib/sessions/backends/cache.py#L13 So some overrides are needed and a custom class is required if you want to use django-user-sessions with cached based sessions. That is why there is a PR. However, some refactoring on the right parent class would simplify the PR. Only |
The goal of this package is to provide Session models as first-class ORM models. This allows to query the session store (database) by user, to see all (in)active sessions by a user. Or to list all users with an active session. Using a cache backend, I cannot see how these features can be provided? |
O love this package, and this is a great PR for better use of session, please, make a effort to make this unit tests. |
Is there likely to be any more movement on this PR? Is it worth resurrecting the work on here, or would it be best to start a new fork? I'm very interested in the functionality of this package, but I need the speed of a cache based session backend, as opposed to database backed. |
@tarkatronic I would be happy to merge useful session backends into this package, however I'm not convinced adding a cache backend is in line with this package's goal. See my earlier remarks above. |
@Bouke Since you mentioned it's not one of the main goals... IMO, this should be one of the priorities. If your goal is to provide an alternative middleware & all other session infrastructures as opposed to the Django's default one, I think it's essential to have these kinds of options in backend for django-user-sessions to be used on production. If there are a fairly large number of active users that are using the product, I'm concerned that this can be a serious performance issue. Which is the reason why I deferred using this... I can continue this work if the original PR author is absent. Tell me what you think. |
I'm still not seeing how a cache backend can be queried for a list of all sessions, or user's sessions, as I challenged the original submitter before (without nasty hacks). So if I'm missing something, please enlighten me. Otherwise working on the PR would be a waste of time. |
@Bouke oh, no. I haven't looked into how the current PR is implemented, I'm talking about its necessity in general since you explicitly mentioned that it's not one of the main goals. I'm talking about the equivalent of Since this library has the db backend, cached_db should be fairly easy to implement. https://github.com/django/django/blob/master/django/contrib/sessions/backends/cached_db.py |
@stewartpark @Bouke We have implemented a customized |
Hi,
I added the cache backend for users who use the cache as session backend. It could be a lil bit refactor with the db backend.
It would be nice if you can merge it :)
Best regards,
Alberto