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
Fixed #29867 -- Added support for storing None value in caches. #13671
Conversation
2510b0b
to
945179b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pope1ni Thanks 👍 This looks great 🚀
945179b
to
e6278dc
Compare
Thanks for the review. Updated. |
e6278dc
to
f70e78c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pope1ni Thanks 👍 I pushed minor edits to tests and backends. I'm going to check docs tomorrow.
f70e78c
to
0d4cfd7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pushed small edits to docs.
Many of the cache operations make use of the default argument to the .get() operation to determine whether the key was found in the cache. The default value of the default argument is None, so this results in these operations assuming that None is not stored in the cache when it actually is. Adding a sentinel object solves this issue. Unfortunately the unmaintained python-memcached library does not support a default argument to .get(), so the previous behavior is preserved for the deprecated MemcachedCache backend.
0d4cfd7
to
bb64b99
Compare
Fixes ticket-29867, supersedes #11812 and #13055
This is following on from my comments in #11812 in particular.
Many of the cache operations make use of
get()
with the implicitdefault=None
and assume that the returnedNone
value indicates the value is not stored. This may not be true asNone
can be explicitly set into the cache. The fix is to use a sentinel object as the default in the absence of the key in the cache.Unfortunately this does not work for python-memcached, so we introduce a feature flag to keep the existing behaviour for that backend.