Opened 7 years ago
Last modified 5 years ago
#29867 closed Bug
Allow cache.get_or_set() to cache a None result — at Version 2
| Reported by: | Phill Tornroth | Owned by: | nobody |
|---|---|---|---|
| Component: | Core (Cache system) | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description (last modified by )
get_or_set docstring says "If the key does not exist, add the key and set it to the default value." -- but that's not quite what it does. It will perform a set if the key doesn't exist, or if the cached value is None.
I think in order to be doing what it says on the tin it'd need to be:
if self.has_key(key, version=version):
return self.get(key, version=version)
else:
if callable(default):
default = default()
self.add(key, default, timeout=timeout, version=version)
# Fetch the value again to avoid a race condition if another
# caller added a value between the first get() and the add()
# above.
return self.get(key, default, version=version)
I'd find this useful in cases where None was an expensive result to arrive at. If there's spiritual alignment with the suggestion, I'm happy to prepare and submit a change with tests.
Change History (2)
comment:1 by , 7 years ago
| Summary: | cache.get_or_set won't cache None results → Allow cache.get_or_set() to cache a None result |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
comment:2 by , 7 years ago
| Description: | modified (diff) |
|---|
Note:
See TracTickets
for help on using tickets.
I agree with your analysis. I read through previous related issues (#26792, #28601) and didn't see a good reason for the current behavior. I would question if
if default is not None:should be there in your proposal (i.e. why shouldn'tNoneby cached?).