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

Redis(Cache) now (re)supports function as a key_prefix #332

Closed
wants to merge 0 commits into from

Conversation

@northernSage
Copy link
Member

northernSage commented Feb 10, 2024

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

@@ -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()
Copy link
Member

@northernSage northernSage Feb 10, 2024

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

@northernSage
Copy link
Member

northernSage commented Feb 10, 2024

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! :bowtie:

@dbascoules
Copy link
Author

Thank you for your review and maintaining this library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment