Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/13354 )
Change subject: [tool] Add cluster name resolver for CLI tools ...................................................................... Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/13354/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13354/1//COMMIT_MSG@11 PS1, Line 11: This patch add a feature to allow CLI tools to resolve cluster : name configured in a YAML file ${$KUDU_CONFIG}/cluster_info.yaml. : > Hmm, do we really need this example file? It adds clutter to the top-level OK, I'll remove it and add some comments to introduce how to use it. http://gerrit.cloudera.org:8080/#/c/13354/1/cluster_info.yaml File cluster_info.yaml: PS1: > This file merits a copyright header. Done http://gerrit.cloudera.org:8080/#/c/13354/1/src/kudu/tools/tool_action_common.cc File src/kudu/tools/tool_action_common.cc: http://gerrit.cloudera.org:8080/#/c/13354/1/src/kudu/tools/tool_action_common.cc@503 PS1, Line 503: // master_addresses: ip1:port1,ip2:port2,ip3:port3 > Why not just assume that any string can be a cluster name? If someone decid Done http://gerrit.cloudera.org:8080/#/c/13354/1/src/kudu/tools/tool_action_common.cc@512 PS1, Line 512: auto config_file = string(kudu_config_path) + "/cluster_info.yaml"; > KUDU_HOME is only really useful in development contexts. How about the hier Done http://gerrit.cloudera.org:8080/#/c/13354/1/src/kudu/tools/tool_action_common.cc@513 PS1, Line 513: if (Env::Default()->FileExists(config_file)) { > These CHECKs should be converted into bad Statuses. You'll probably need to Done -- To view, visit http://gerrit.cloudera.org:8080/13354 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2fc235906adbf25fcc2059fa8443e3013830ec01 Gerrit-Change-Number: 13354 Gerrit-PatchSet: 2 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mitch Barnett <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Sat, 18 May 2019 03:46:07 +0000 Gerrit-HasComments: Yes
