Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11036 )
Change subject: hms tools: do not require HMS configuration flags ...................................................................... Patch Set 1: (7 comments) http://gerrit.cloudera.org:8080/#/c/11036/1/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/11036/1/src/kudu/tools/kudu-tool-test.cc@2193 PS1, Line 2193: master_addr nit: master_addrs. And also in all other places. http://gerrit.cloudera.org:8080/#/c/11036/1/src/kudu/tools/kudu-tool-test.cc@2211 PS1, Line 2211: Kerberos is enabled Do we need to parameterized this with Kerberos disabled/enabled? http://gerrit.cloudera.org:8080/#/c/11036/1/src/kudu/tools/tool_action_common.h File src/kudu/tools/tool_action_common.h: http://gerrit.cloudera.org:8080/#/c/11036/1/src/kudu/tools/tool_action_common.h@115 PS1, Line 115: bool all_flags, Can you add a bit explanation/comment on what is the parameter for? And how it would work with 'flag_tags'? http://gerrit.cloudera.org:8080/#/c/11036/1/src/kudu/tools/tool_action_hms.cc File src/kudu/tools/tool_action_hms.cc: http://gerrit.cloudera.org:8080/#/c/11036/1/src/kudu/tools/tool_action_hms.cc@113 PS1, Line 113: FLAGS_hive_metastore_uris.empty() When the URIs and SASL flags will not be empty? Do we need to validate if the flags' value match with the ones in master in this case? http://gerrit.cloudera.org:8080/#/c/11036/1/src/kudu/tools/tool_action_hms.cc@116 PS1, Line 116: master_addrs[0] Is it possible that different masters have different configs? http://gerrit.cloudera.org:8080/#/c/11036/1/src/kudu/tools/tool_action_hms.cc@116 PS1, Line 116: master::Master::kDefaultPort What happens if the port being used is different from the default? http://gerrit.cloudera.org:8080/#/c/11036/1/src/kudu/tools/tool_action_hms.cc@120 PS1, Line 120: hive_metastore_uris > nit: does it make sense to introduce constant literals for 'hive_metastore +1 -- To view, visit http://gerrit.cloudera.org:8080/11036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibf580384a1612174a75142295d7f604cd6fabfe1 Gerrit-Change-Number: 11036 Gerrit-PatchSet: 1 Gerrit-Owner: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 24 Jul 2018 21:52:55 +0000 Gerrit-HasComments: Yes
