chore: add noop docs subcmd #4214
Conversation
| @@ -2464,6 +2466,10 @@ void Service::Command(CmdArgList args, Transaction* tx, SinkReplyBuilder* builde | |||
| return; | |||
| } | |||
|
|
|||
| if (subcmd == "DOCS" && sufficient_args) { | |||
| return builder->SendOk(); | |||
There was a problem hiding this comment.
I don't know about that. Perhaps some libraries will assume we don't support / have some commands if we return none?
Since this was introduced in Redis 7, perhaps we should keep it not implemented, or implement it properly?
@romange wdyt?
There was a problem hiding this comment.
That's what Roman asked on the issue. To make it a noop. I was multitasking while waiting for a test to run and I patched this quickly
There was a problem hiding this comment.
quoting Roman
we should make it a noop - and reply OK
There was a problem hiding this comment.
Yes, I saw that reply :)
I'm not sure if Roman specifically considered the downside of perhaps breaking some clients, that's why I raised it
There was a problem hiding this comment.
Ohhhh ok ok. We did that with other commands in the past so I guess it's fine 🤷♂️
But +1 that you mentioned/asked
There was a problem hiding this comment.
I am not aware of clients that rely on it but in theory it can be used to provide dynamic support for commands.
There was a problem hiding this comment.
Shall I close or merge this ? 🤷
There was a problem hiding this comment.
merge it. let's learn about universe experimentally.
Fixes #4200