-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
Pass in cache timeout for async queries #4436
Conversation
8d182d4
to
8d6123e
Compare
LGTM |
cache_timeout = database.cache_timeout | ||
if cache_timeout is None: | ||
cache_timeout = config.get('CACHE_DEFAULT_TIMEOUT', 0) | ||
results_backend.set(key, utils.zlib_compress(json_payload), cache_timeout) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh interesting. FYI we use the s3 caching as a result backend which doesn't really support timeout, so this won't matter on our end, but for people using other results backend that's probably the desired behavior. https://github.com/bkyryliuk/s3werkzeugcache/blob/master/s3cache/s3cache.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I figured that may have been the case.
To give some more context on our end, we use twemcache as our results backend, and, depending on which client is used, defaulting to None
can cause issues.
To conclude, since the werkzeug.contrib.cache.BaseCache
's set
method includes a timeout
field, it should be passed down through here as well.
* Pass in cache timeout for async queries * Default cache timeout to 0 if default env var is not set * check for 0 timeout
* Pass in cache timeout for async queries * Default cache timeout to 0 if default env var is not set * check for 0 timeout
This allows cache timeout for asynchronous queries to be tuned on a per-database level.
@mistercrunch