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

RedisCache #150

Merged
merged 1 commit into from
Aug 2, 2019
Merged

RedisCache #150

merged 1 commit into from
Aug 2, 2019

Conversation

chrisrossi
Copy link
Contributor

Concrete implementation of GlobalCache which uses Redis. Thanks to @takashi8

Concrete implementation of ``GlobalCache`` which uses Redis.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 2, 2019
@chrisrossi chrisrossi requested a review from cguardia August 2, 2019 19:48
Copy link
Contributor

@andrewsg andrewsg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your extensive work on this.

"""Implements :meth:`GlobalCache.delete`."""
self.redis.delete(*keys)

def watch(self, keys):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is watch() part of the original ndb's spec for caching? I don't understand how they made this work with the memcached api.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

watch and compare_and_swap are equivalent to using get and set in memcached using CAS tokens. Had to generalize the concept a bit to work with Redis. I also did away with the CAS acronym because it's a bit opaque. (I had to do some Googling when I first encountered it.)

@andrewsg
Copy link
Contributor

andrewsg commented Aug 2, 2019

Looks like we have a systems test error, but I'm unclear if it's affected by this PR or due to something else.

@chrisrossi chrisrossi added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 2, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 2, 2019
@chrisrossi
Copy link
Contributor Author

System test error seems to have been transient, related to eventual consistency. We've managed to squash most of those, but one still gets through sometimes.

Copy link
Contributor

@cguardia cguardia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great.

@chrisrossi chrisrossi merged commit 865ca8a into googleapis:master Aug 2, 2019
@chrisrossi chrisrossi deleted the redis-cache branch January 12, 2020 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants