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

Fix for deletes not persisting. #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mackstann
Copy link

We've had a problem where deleting session items, particularly flash messages, does not seem to work in some environments. In our dev and staging environments, we see this bug, where the flash messages pile up from pageview to pageview and don't want to go away. For some reason it isn't an issue in production; I'm not sure why. In any case, when I found what appeared to be causing the bug, it seemed fairly clear to me that it is a bug. The triggering of channel.backend_write() happens with a set, but not with a del. So I've added that call in __delitem__ to make sure it persists its changes. I refactored the conditional call to backend_write into a named method just to avoid duplicating the accompanying comment. I also made this call to backend_write optional when doing a delete, because set actually does a delete first, and there is no point in set doing two writes.

So far this has only been manually tested by me.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 859b341 on cartlogic:delete_persistence_bug into d381849 on storborg:master.

@storborg
Copy link
Owner

Can you post the pip freeze from your dev environment?

I think this introduces unnecessary backend writes when deleting a client-only key-- it probably just needs a conditional when calling channel.ensure_backend_write().

@mackstann
Copy link
Author

Here's pip freeze from my dev instance: https://gist.github.com/mackstann/f208756fe52faa2b5611

And here's pip freeze from another instance on our QA staging server where the problem also occurs: https://gist.github.com/mackstann/f83bd2b3940fcecef995

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

Successfully merging this pull request may close these issues.

None yet

3 participants