sqlalchemy / sqlalchemy/dogpile.cache

cache_on_arguements get does not respect expiration time

Open
#152 10 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

caching decorator documentation easy invalidation region
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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.