Support Redis Cluster

[lewispham]

This PR is for supporting Redis Cluster in catbox-redis. I'm using the image docker-redis-cluster for running the related integration tests locally.

There is a caveat about this PR. Testing authentication of redis cluster can not be included because docker-redis-cluster does not support building clusters along with passwords. Related issue Grokzen/docker-redis-cluster#54

Please check README.md and examples/redis-cluster.js for more details about how to use and test this PR.

[hueniverse]

Is this the same as #84? Want to make sure they are solving the same problem as this one seems to be further along.

@marcuspoehls Got time to review?

[marcuspoehls]

@lewispham @hueniverse Hey Lewis, Hey Eran, I’ll take time this week and have it reviewed until Friday.

[marcuspoehls]

@lewispham Hi Lewis, I read your code and reviewed the changes (didn’t make any comments yet).

You updated a lot of code and I’m not sure if it’s necessary. I thought that we could extend the existing start() function of catbox-redis with support for a client that connects to a Redis cluster. The rest of the existing functionality would be the same because it uses the connected client instance to interact with Redis and Redis Cluster.

Here’s a PR for that: https://github.com/hapijs/catbox-redis/pull/95/files

What do you think about my linked PR?

[lewispham]

@marcuspoehls There are some issues with your PR.

• No integration tests. I personally think that it's better to have integration tests for redis cluster.
• Not enough tests for redis cluster. Your PR does not test catbox methods when client is redis cluster.
• The syntax is not quite straight forward. Most users are familiar with ioredis so I think it's better to follow their way of constructing redis cluster.
• No example of using redis cluster

They're just my personal sense though. It's actually better to use your approach if you don't want to add more complexity to this lib.

[lock]

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.