[email protected] has posted comments on this change. ( http://gerrit.cloudera.org:8080/19938 )
Change subject: KUDU-3477 Function to get flags of minicluster daemons. ...................................................................... Patch Set 6: (3 comments) http://gerrit.cloudera.org:8080/#/c/19938/1/src/kudu/mini-cluster/external_mini_cluster.cc File src/kudu/mini-cluster/external_mini_cluster.cc: http://gerrit.cloudera.org:8080/#/c/19938/1/src/kudu/mini-cluster/external_mini_cluster.cc@103 PS1, Line 103: using kudu::server::GetFlagsRequestPB; Is it ok to expect proto messages as the function parameters, or should this structures be duplicated with a plain c++ object? http://gerrit.cloudera.org:8080/#/c/19938/3/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/19938/3/src/kudu/tools/kudu-tool-test.cc@7803 PS3, Line 7803: auto* r = req.mutable_get_daemon_flags(); : *r->mutable_id() = tservers[0].id(); : r->mutable_request()->add_flags("rpc_negotiation_timeout_ms"); > If check failed, gtest will output a message like: Thank you. http://gerrit.cloudera.org:8080/#/c/19938/3/src/kudu/tools/tool.proto File src/kudu/tools/tool.proto: http://gerrit.cloudera.org:8080/#/c/19938/3/src/kudu/tools/tool.proto@213 PS3, Line 213: // The actual request : optional kudu.server.GetFlagsRequestPB request = 2; : } : : // Call SetFlag() on the specific daemon. : message SetDaemonFlagRequestPB { : // The identifier of the d > It's duplicate to server_base.GetFlagsResponsePB, is it possible to reuse G It is possible, but it will include a dependency of tools_proto -> server_base_proto. That's why I wanted to duplicate it. If this new dependency is acceptable, then it indeed makes the changes smaller. I don't know if introducing this dependency has any negative consequences, probably not. I updated the code review. I can revert the proto back if the dependency is bad. -- To view, visit http://gerrit.cloudera.org:8080/19938 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3480bfccce18c4605ed9b4ed1dab367fa1fd525a Gerrit-Change-Number: 19938 Gerrit-PatchSet: 6 Gerrit-Owner: Anonymous Coward <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Comment-Date: Fri, 16 Jun 2023 10:55:02 +0000 Gerrit-HasComments: Yes
