search

StackExchange / StackExchange.Redis

General purpose redis client
https://stackexchange.github.io/StackExchange.Redis/
Other
5.87k stars 1.51k forks source link

Feature request - add command: ROLE #1451

Closed NickCraver closed 3 years ago

NickCraver commented 4 years ago

This command isn't yet in the lib and is useful for a few scenarios - came up in #1431.

Min version: 2.8.12 Docs: https://redis.io/commands/role

zmj commented 4 years ago

I would like to take a shot at this! A couple questions:

mgravell commented 4 years ago

Given that the shape changes so radically, inheritance is probably the best option here, agreed. There is also an enum for the same purpose, but to be honest I don't think we need the discrimiant once we have split it into types. The intent is also perfectly valid in primary/replica scenarios, so all 3 result types probably need testing. We should also consider what to return when the type isn't recognised ... an exception? A RoleUnknown subtype? I'd also recommend looking for the string "replica" in addition to "slave", in case that ever swaps over.

Looking forward to it!

On Fri, 14 Aug 2020, 23:54 zmj, notifications@github.com wrote:

I would like to take a shot at this! A couple questions:

  • Result type: it seems like there's not much shared between the results for each node type. I was thinking to return a base Role type with some subtypes for master, replica, and sentinel?
  • Tests: it seems like the use case for this command is mostly around configurations with sentinels? Is that a sensible place to put tests for this command?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/StackExchange/StackExchange.Redis/issues/1451#issuecomment-674302845, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAEHME5DPCU7HXC3DFCOA3SAW6BZANCNFSM4MXUI6CA .

NickCraver commented 3 years ago

Added in #1551, closing out!