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 cluster commands #685

Merged
merged 3 commits into from
Feb 19, 2016
Merged

Redis cluster commands #685

merged 3 commits into from
Feb 19, 2016

Conversation

iandyh
Copy link
Contributor

@iandyh iandyh commented Dec 1, 2015

In some cases, you still need redis-py to perform cluster related commands, e.g. slaves, info, replicated. This PR implements all the cluster commands (cluster sub-command args).

Because of the special reply from Redis Cluster, the tests mock out all the responses from Redis server and test it against mock response.

Cheers.

@iandyh
Copy link
Contributor Author

iandyh commented Dec 1, 2015

Currently the CI fails because the lengthy mocked Redis raw response breaks the pep8 check. I prefer to keep the raw response as it is with an ignore comment. I could also break down the long string into tuples and then join them later. It's up to maintainer's decision.

@iandyh
Copy link
Contributor Author

iandyh commented Jan 13, 2016

@andymccurdy Hi, is there a timetable for review the PR? It is quite essential for anyone who is running Redis Cluster.

@iandyh
Copy link
Contributor Author

iandyh commented Feb 19, 2016

@andymccurdy Ping. I've pep8ed the code and please review the PR.

@andymccurdy
Copy link
Contributor

Hey, sorry for the delay. This seems mostly fine. It might be nice to create separate methods for each cluster command (similar to how the sentinel commands are implemented.

I'll go ahead and merge this anyway. Thanks for doing this.

andymccurdy added a commit that referenced this pull request Feb 19, 2016
@andymccurdy andymccurdy merged commit b40875d into redis:master Feb 19, 2016
@iandyh
Copy link
Contributor Author

iandyh commented Feb 19, 2016

@andymccurdy That was quick. Thanks for the merge! I'll consider the advice. Current design aligns with the cluster command in ruby client.

@iandyh
Copy link
Contributor Author

iandyh commented Mar 8, 2016

@drabaioli Hi! @Grokzen has explained most of the things. I'll add a bit more. Basically, even though redis-py-cluster provides most of the features you need to talk to Redis Cluster, you still need the ability to talk to one specific node inside the cluster. e.g. cluster replicate node-id will configure a node as a slave of another node and clearly you don't want to send this kind of command to every node in the cluster.

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.

4 participants