Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9948 )

Change subject:  Add GetFlags endpoint and tool
......................................................................


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/9948/1/src/kudu/server/generic_service.cc
File src/kudu/server/generic_service.cc:

http://gerrit.cloudera.org:8080/#/c/9948/1/src/kudu/server/generic_service.cc@74
PS1, Line 74: void GenericServiceImpl::GetFlags(const GetFlagsRequestPB* req,
> Could you also test this independently of the CLI?
Done


http://gerrit.cloudera.org:8080/#/c/9948/1/src/kudu/server/generic_service.cc@99
PS1, Line 99:     flag->set_is_user_set(!entry.second.is_default);
            :     flag->set_is_default(entry.second.current_value == 
entry.second.default_value);
> Having a hard time seeing the distinction between these two, because I had
Mmm yeah probably best.


http://gerrit.cloudera.org:8080/#/c/9948/1/src/kudu/server/server_base.proto
File src/kudu/server/server_base.proto:

http://gerrit.cloudera.org:8080/#/c/9948/1/src/kudu/server/server_base.proto@47
PS1, Line 47: flags match.
> This doesn't make sense; did your thought get cut off?
A word got eaten: "all flags match".


http://gerrit.cloudera.org:8080/#/c/9948/1/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/9948/1/src/kudu/tools/kudu-tool-test.cc@2415
PS1, Line 2415: TEST_F(ToolTest, TestGetFlags) {
> Maybe also test the tserver's endpoint?
Done


http://gerrit.cloudera.org:8080/#/c/9948/1/src/kudu/tools/tool_action_common.h
File src/kudu/tools/tool_action_common.h:

http://gerrit.cloudera.org:8080/#/c/9948/1/src/kudu/tools/tool_action_common.h@110
PS1, Line 110: Status GetServerFlags(const std::string& address, uint16_t 
default_port);
> This isn't quite symmetric with SetServerFlag since it doesn't return the f
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia35b4261099c1a3c6e2ff68e907c84df9a7ff699
Gerrit-Change-Number: 9948
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wdberke...@gmail.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com>
Gerrit-Comment-Date: Sat, 07 Apr 2018 06:58:47 +0000
Gerrit-HasComments: Yes

Reply via email to