Skip to content
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

region.get does not respect object cache #143

Closed
jasonkuhrt opened this issue Feb 19, 2019 · 6 comments
Closed

region.get does not respect object cache #143

jasonkuhrt opened this issue Feb 19, 2019 · 6 comments

Comments

@jasonkuhrt
Copy link

import time
from pprint import pprint

from dogpile.cache.region import make_region

region = make_region().configure("dogpile.cache.memory")

region.get_or_create("foo-key", lambda: "foo-value", expiration_time=2)

time.sleep(1)

result_1 = region.get("foo-key")

# should be hit, ok
pprint(dict(result_1=result_1))

time.sleep(2)

result_2 = region.get("foo-key")

# should be miss, fail
pprint(dict(result_2=result_2))
❯ python yolo.py
{'result_1': 'foo-value'}
{'result_2': 'foo-value'}

I expected something like

❯ python yolo.py
{'result_1': 'foo-value'}
NO_VALIUE
@zzzeek
Copy link
Member

zzzeek commented Feb 19, 2019

result_2 = region.get("foo-key")

should be miss, fail

pprint(dict(result_2=result_2))

the value is not expired because you have not specified an expiration time.

call it like this:

result_2 = region.get("foo-key", expiration_time=2)
then you get:

{'result_1': 'foo-value'}
{'result_2': <dogpile.cache.api.NoValue object>}

so it would appear I need to add a note to the "expiration_time" argument to get() that this is a per-get expiration time, it is not stored with the object. dogpile doesn't store per-object expiration times.


❯ python yolo.py
{'result_1': 'foo-value'}
{'result_2': 'foo-value'}


I expected something like

❯ python yolo.py
{'result_1': 'foo-value'}
NO_VALIUE

@jasonkuhrt
Copy link
Author

@zzzeek thanks for the very prompt reply!

the value is not expired because you have not specified an expiration time.

Ok, so it seems we misunderstood what expirationn_time in region.get_or_create("foo-key", lambda: "foo-value", expiration_time=2) was doing.

dogpile doesn't store per-object expiration times

Ah, our use-case requires per-object expiration times (JWT token exp value) so evidently we should not be using dogpile then.

@hugoduncan
Copy link

@zzzeek would you contemplate adding an expiration_fn on get and get_or_create, that is called with any cached value as an argument, to determine if the value has expired or not? The use case is for when the object itself has an expiration time, e.g. a JWT token.

@zzzeek
Copy link
Member

zzzeek commented Feb 20, 2019

@hugoduncan this rings a bell, like we've proposed this somewhere. let me check

@zzzeek
Copy link
Member

zzzeek commented Feb 20, 2019

there's a bunch of issues that are all still open for discussion that get into this, see #124, #125, and others they refer towards. There's a lot of issues that are just open in this area like #45, etc. there is a pluggable invalidation strategy also that was added in #38 but doesn't work on a per item basis (should it?). We do have a "should_cache_fn" callable , but for invalidation there's two kinds of invalidation too, which is what RegionInvalidator gets at.

Coming up with APis that allow all these different things to be possible without them conflicting with each other, missing out on key functionality that is impossible to add withou breaking APIs (such as, hard vs. soft invalidation) or introducing new bugs is really hard, and I generally don't have the resources to attend to dogpile.cache other than passing through contributions. If you can take a look at those above issues maybe some sense can be made from them for a feature that we can all work with.

@hugoduncan
Copy link

Thanks for taking the time to put together the thorough summary above. What you've described sounds like a good approach, but I don't think we'll have time to do such a broad update. Look forward to seeing if this gets picked up in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants