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

Flexible connections for sentinel support #429

Closed
wants to merge 3 commits into from
Closed

Flexible connections for sentinel support #429

wants to merge 3 commits into from

Conversation

benbuckman
Copy link

This supports redis-sentinel-client. The changes are mostly cherry-picked from tanguylebarzic's fork and our/DocuSignDev's fork.

There is a discussion about how to support sentinels here: #302
The idea of supporting pluggable connection managers is mentioned there, and may be more robust. I'm submitting this PR so the changes needed for my sentinel implementation are documented.

Thank you.

Ben Buckman added 3 commits April 30, 2013 12:29
…s (for sentinel).

(cherry-picking f34fde2 from tanguylebarzic)
not sure if necessary
(cherry-picking a281402 from tanguylebarzic)
not sure if necessary
@brycebaril
Copy link
Contributor

Working on digesting this. One thing I'm hoping to do with pluggable connection managers is deal with the RedisClient options bloat that has been happening. We keep adding new options, which is a bit of a maintenance and documentation nightmare.

This patch adds two (disable_flush and allowNoSocket right? -- also, notice the casing inconsistency) new options that aren't documented, and may not provide useful behavior when the 3rd-party client that uses them isn't in use.

@philjackson
Copy link

Has there been any movement on this? Would be really good to get something into NPM that works.

@justinfreitag-zz
Copy link

bump

@analytically
Copy link

👍

@benbuckman benbuckman closed this Apr 26, 2016
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.

5 participants