sqlalchemy / sqlalchemy/dogpile.cache
cache_on_arguements get does not respect expiration time
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 299
- Forks
- 50
- PR merge metrics
- No merged PRs in 30d
Description
The get method attached by cache_on_arguements does not respect the expiration_time passed to cache_on_arguments.
This leads to potentially bad behavior that decorated_method.get()does not return NO_VALUE when the cached value is older than expiration_time, and the value it does return is different from the value that would have been retrieved by decorated_method(). I can see this leading to quite tricky to solve bugs if the two ways of accessing the cache were used across a codebase.
If this is unintended behaviour then I would suggest decorated_method.get pass expiration_time to self.get. Potentially it may also be wise to add a parameter to cache_on_arguements to ignore expiration_time on get since that is a reasonable use case.
If intended it should be explicitly stated in the documentation to avoid confusion.
Example:
import datetime
from time import sleep
from dogpile.cache import make_region
my_region = make_region().configure('dogpile.cache.memory')
@my_region.cache_on_arguments(expiration_time=1)
def get_cached_datetime():
return datetime.datetime.now()
get_cached_datetime() # Returns current time (DateTime 1)
sleep(1)
get_cached_datetime.get() # Returns DateTime1 even though cache value should be invalidated
get_cached_datetime() # Returns current time (DateTime 2)
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at the implementation of cache_on_arguments and the decorated_method.get path, then reproduce the supplied example with the memory backend and a one-second expiration. Done means get() treats an expired cached value consistently with the decorated method and returns NO_VALUE when appropriate; check whether existing tests cover both access paths.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100