-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Added CommandMap check before requesting cluster nodes #2014
Conversation
Interesting that this failed with Envoy proxy. It looks like Envoy does actually support RedisCommand.CLUSTER even though their documentation doesn't list it. So should I remove that from the exclusions here? StackExchange.Redis/src/StackExchange.Redis/CommandMap.cs Lines 48 to 78 in 6718fea
|
@tylerohlsen this wasn't you - had a fix pending in #2011. Just merged to bringing main to your branch should help. I think for this PR we should just add a test with it disabled, e.g. a |
I certainly can. That might take me a bit longer. |
@tylerohlsen I gotcha - pushing up a test and release notes! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, thanks @tylerohlsen!
Awesome! Thanks @NickCraver |
Fixes #2012