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

timeout=None in django 1.6 #47

Closed
rmmr opened this issue Nov 28, 2013 · 7 comments
Closed

timeout=None in django 1.6 #47

rmmr opened this issue Nov 28, 2013 · 7 comments

Comments

@rmmr
Copy link
Contributor

rmmr commented Nov 28, 2013

Just noticed and by looking at the master, django-redis does not fully support django 1.6 . Previously, passing None explicitly would use the default timeout value. Now it will cache the value forever. This change was made in order to allow timeout = 0 to let the cache expire immediately. I'll most likely make a fork, cause I need this badly.

@niwinz
Copy link
Collaborator

niwinz commented Nov 29, 2013

Making this change is dangerous, because, previously, the permanent cache is done by setting timeout to 0 and a new version just do the opposite behavior. All behavior that are now supported on django 1.6, are already supported much earlier in django-redis.

I don't know the good way to change this mantaining backward compatibility.

@rmmr
Copy link
Contributor Author

rmmr commented Nov 29, 2013

I think it's quite simple. Check the django version and use a settings.py entry. For 1.6 and up switch to the behaviour as documented in django 1.6. This means timeout = 0 will not cache indefinitely for django 1.6 users. If by any chance 1.6 users will want to keep the old behaviour, they can change the settings.py entry.

It's dangerous not to make any change, because 1.6 users who want to switch from django's default caching backends to yours can run into trouble.

I'll work this out in my fork anyways. Cheers!

@niwinz
Copy link
Collaborator

niwinz commented Nov 29, 2013

Great! Thanks!

@rmmr
Copy link
Contributor Author

rmmr commented Nov 29, 2013

Allright, made an initial attempt to fix this. Changes can be found in client/default.py at lines 32 and 140 - 145. Changes should be self-explanatory.

@niwinz
Copy link
Collaborator

niwinz commented Dec 12, 2013

Sorry, but I don't have much time this week for it. This weekend this should be merged and fixed!

@niwinz
Copy link
Collaborator

niwinz commented Dec 15, 2013

Now partially merged with minor changes: 09939dc

Backward compatibility changes are not necessary, because 0 immediate timeout is only for memcached backend and django-redis can continue with own behavior :D

Thanks for your efforts!

@niwinz niwinz closed this as completed Dec 15, 2013
@rmmr
Copy link
Contributor Author

rmmr commented Dec 15, 2013

Thank you too! Even better :) who needs immediate timeouts anyway.

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

2 participants