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

Allow to set custom key in RedisCacheHandler #761

Merged
merged 3 commits into from Dec 22, 2021
Merged

Allow to set custom key in RedisCacheHandler #761

merged 3 commits into from Dec 22, 2021

Conversation

ENT8R
Copy link
Contributor

@ENT8R ENT8R commented Dec 22, 2021

I would like to use a different key than the one that is always set, so I thought it might be a good idea to allow users of the RedisCacheHandler to supply their own key and only set it to the default value of token_info if the key is not supplied as an argument.

@stephanebruckert
Copy link
Member

Looks good, please just add a changelog

@ENT8R
Copy link
Contributor Author

ENT8R commented Dec 22, 2021

I added the changes to the changelog and I also added another check before trying to retrieve the value to make sure that the key exists (already noticed by @Findus23 in #747 (review))

@stephanebruckert
Copy link
Member

Thank you!

@stephanebruckert stephanebruckert merged commit 08411b9 into spotipy-dev:master Dec 22, 2021
@ENT8R ENT8R deleted the patch-1 branch December 22, 2021 15:02
@Findus23
Copy link

Sorry, to nitpick once again, but isn't something like

token_info = self.redis.get('token_info')
if token_info:
    return json.loads(token_info)

easier as it only does one request to redis instead of two (and doesn't have a really unlikely race-condition)?

@stephanebruckert
Copy link
Member

@Findus23 that's fair!

IdmFoundInHim pushed a commit to IdmFoundInHim/spotipy that referenced this pull request Dec 23, 2021
* Allow to set custom key in RedisCacheHandler

* Update CHANGELOG.md

* Check for the existence of the key
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

Successfully merging this pull request may close these issues.

None yet

3 participants