Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16642 )

Change subject: [tools] mention whitespace as a separator for variadic args
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16642/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16642/2//COMMIT_MSG@10
PS2, Line 10: As it turned out, the
            : help/usage message
> In this particular case, was the user actually checking the help message to
I don't know whether they looked into the help message or not after getting the 
'Invalid argument' error.  But that's what I did when they reported the issue :)

I guess they saw the following error message:

Invalid argument: Peer with uuid A,B is not in the committed  config on this 
replica, rejecting the unsafe config change request for tablet T. Committed 
config: opid_index: -1 OBSOLETE_local: true peers { permanent_uuid: "UUID" 
member_type: VOTER last_known_addr { host: "a.b.c.d" port: PPPP } }

Yep, I guess the tool could first try to verify the args before issuing an RPC, 
but this is not what it does right now.

Anyways, I think it doesn't hurt to be more specific when describing the 
arguments, especially when we have other CLI commands with arguments expected 
to be comma-separated.  Maybe, at some point we should switch to 
comma-separated format for all arguments having the list-like semantics?

Thank you for review!



--
To view, visit http://gerrit.cloudera.org:8080/16642
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ef979bd17655b5d46e1adb2a2f22923353ac435
Gerrit-Change-Number: 16642
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 24 Oct 2020 00:35:32 +0000
Gerrit-HasComments: Yes

Reply via email to