Opened 6 years ago
Last modified 4 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 , 6 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 , 6 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'tNone
by cached?).