Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13354 )
Change subject: [tool] Add cluster name resolver for CLI tools ...................................................................... Patch Set 5: (3 comments) BTW, now that this is wrapping up we should further document the layout and semantics of the YAML-based client config file. Would you like to do that in a follow-up patch? http://gerrit.cloudera.org:8080/#/c/13354/4/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/13354/4/src/kudu/tools/kudu-tool-test.cc@893 PS4, Line 893: kTestCopyTableUpsert, : kTestCopyTableSchemaOnly, : kTestCopyTableComple > There is an assert "CHECK_EQ(1, mini_masters_.size());" in master_proxy(), Makes sense, thanks for the explanation. http://gerrit.cloudera.org:8080/#/c/13354/4/src/kudu/tools/kudu-tool-test.cc@4644 PS4, Line 4644: ASSERT_TRUE(s.IsRuntimeError()); > No, this is to cleanup existing environment variable, e.g. my own developin Yeah but that's unique to your environment, right? Why should it be checked into the test? http://gerrit.cloudera.org:8080/#/c/13354/5/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/13354/5/src/kudu/tools/kudu-tool-test.cc@619 PS5, Line 619: Corrput Corrupt -- 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: 5 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: Todd Lipcon <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Tue, 28 May 2019 18:26:08 +0000 Gerrit-HasComments: Yes
