-
-
Notifications
You must be signed in to change notification settings - Fork 44
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
Redis(Cache) now (re)supports function as a key_prefix #332
Conversation
Hey @dbascoules, thanks for taking the time to open the PR. Changes seem good overall, we are just short of a test case covering the new feature, would you mind adding one? (just let me know if you can't and I'll pick it up, no worries). Oh, and also please add a change log entry + update docs |
src/cachelib/redis.py
Outdated
@@ -57,6 +57,11 @@ def __init__( | |||
self._read_client = self._write_client = host | |||
self.key_prefix = key_prefix or "" | |||
|
|||
def _get_prefix(self): | |||
return ( | |||
self.key_prefix if isinstance(self.key_prefix, str) else self.key_prefix() |
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.
Not really an issue here, but I wonder if using https://docs.python.org/3/library/functions.html#callable wouldn't make more sense semantically. In the end we want to call the key if it's callable and use it as is if it's not
Just decided to go ahead and do the missing requested changes since it's all minor stuff, so closing in favour of #348 with a more descriptive branch naming. Thanks again for the contribution! |
Thank you for your review and maintaining this library. |
Behaviour added here : RedisCache now supports function as a key_prefix flask-caching#109
Behaviour removed here : https://github.com/pallets-eco/flask-caching/pull/308/files#diff-d15a59e5c66734a6b6d638efe7582b3a147d5a4213dc3b3ca4e3672c50607ae1
fixes Redis doesn't supports function as a key_prefix anymore #331
fixes key_prefix can support func ? : Expected type 'str', got '() -> str' instead flask-caching#536
fixes RedisCache support for callable key prefixes was removed as of 1.11.0 flask-caching#529
fixes RedisCache lost support for function as a key_prefix apache/superset#27077