Implement ROLE command #1551
Implement ROLE command #1551
Conversation
| @@ -1042,14 +1055,5 @@ internal static class IServerExtensions | |||
| /// </summary> | |||
| /// <param name="server">The server to simulate failure on.</param> | |||
| public static void SimulateConnectionFailure(this IServer server) => (server as RedisServer)?.SimulateConnectionFailure(); | |||
|
|
|||
| public static string Role(this IServer server) | |||
There was a problem hiding this comment.
Removed the existing test helper and changed the usage to use the new code
| /// <summary> | ||
| /// This replica's replication state. | ||
| /// </summary> | ||
| public string State { get; } |
There was a problem hiding this comment.
This could be an enum, but returning the raw value seemed better for allowing the caller to do something useful with an unrecognized value.
| public void ReplicaRole() | ||
| { | ||
| var connString = $"{TestConfig.Current.ReplicaServerAndPort},allowAdmin=true"; | ||
| using var muxer = ConnectionMultiplexer.Connect(connString); |
There was a problem hiding this comment.
Is it safe to assume there is always a replica in the test run?
|
A few tests failed. This one looks like a bug: The others looked like network errors that were probably unrelated to this change. |
|
On the failing test: look at public static CommandMap Sentinel
…On Sat, 15 Aug 2020, 21:00 zmj, ***@***.***> wrote:
A few tests failed. This one looks like a bug: StackExchange.Redis.RedisCommandException
: This operation has been disabled in the command-map and cannot be used:
ROLE. I guess I need to add ROLE to one (or more) of the lists in
CommandMap? Which ones are appropriate?
The others looked like network errors that were probably unrelated to this
change.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#1551 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAEHMCMKMN3LYMXXRKJ7NDSA3SNHANCNFSM4QANWCMQ>
.
|
|
I merged in the main branch's test changes. MultiMaster.TestMultiNoTieBreak is still failing. Doesn't seem related to this PR's changes. Any suggestions? |
|
Ignore that one. I'll look into it.
…On Sat, 12 Sep 2020, 01:41 zmj, ***@***.***> wrote:
MultiMaster.TestMultiNoTieBreak [5s 36ms] is still failing. Doesn't seem
related to these changes. Any suggestions?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1551 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAEHMHDQRIN35RI54D4XLDSFK7SVANCNFSM4QANWCMQ>
.
|
This PR implements the ROLE command (#1451): https://redis.io/commands/role#sentinel-output . Redis implementation: https://github.com/redis/redis/blob/unstable/src/replication.c#L2672
Because the result format is pretty different for each instance type, we return a base Role type that the caller has to type check to get to the values specific to the instance type.
I've tested the master and replica cases locally. I haven't tested sentinel, but that one is the most straightforward.