Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/8312 )
Change subject: KUDU-2191 (4/n): WIP: Hive Metastore catalog manager integration ...................................................................... Patch Set 3: (14 comments) This is going to be significantly reworked in the near future. Leaving open for now because I do want to integrate the feedback. http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/integration-tests/master_hms-itest.cc File src/kudu/integration-tests/master_hms-itest.cc: http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/integration-tests/master_hms-itest.cc@58 PS2, Line 58: using hms::HmsClient; > warning: using decl 'KuduTable' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/integration-tests/master_hms-itest.cc@65 PS2, Line 65: class MasterHmsTest : public KuduTest { > warning: using decl 'MiniHms' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/integration-tests/master_hms-itest.cc@67 PS2, Line 67: public: > warning: using decl 'set' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/integration-tests/master_hms-itest.cc@71 PS2, Line 71: void SetUp() override { > warning: using decl 'Split' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/integration-tests/master_hms-itest.cc@72 PS2, Line 72: KuduTest::SetUp(); > warning: using decl 'Substitute' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/integration-tests/master_hms-itest.cc@112 PS2, Line 112: > warning: passing result of std::move() as a const reference argument; no mo ??? http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/master/hms_catalog.cc File src/kudu/master/hms_catalog.cc: http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/master/hms_catalog.cc@43 PS2, Line 43: using strings::CharSet; > warning: using decl 'function' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/master/hms_catalog.cc@47 PS2, Line 47: > warning: using decl 'Substitute' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/master/hms_catalog.cc@54 PS2, Line 54: > warning: initializer for member 'lock_' is redundant [readability-redundant Done http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/master/hms_catalog.cc@161 PS2, Line 161: rpc.callback = s.AsStdStatusCallback(); > warning: missing username/bug in TODO [google-readability-todo] Done http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/master/hms_catalog.cc@182 PS2, Line 182: table->parameters = { > warning: parameter 'schema' is unused [misc-unused-parameters] this will be used in a later rev once were handling columns correctly. http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/master/hms_catalog.cc@250 PS2, Line 250: } > warning: the parameter 's' is copied for each invocation but only used as a Done http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/hms_catalog.cc File src/kudu/master/hms_catalog.cc: http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/hms_catalog.cc@203 PS3, Line 203: for (int idx = 0; idx < table.size(); idx++) { > I feel like this code could be written more concisely using some of the stu perhaps we should just do the split on '.', and send the two parts to hive to validate. We would probably just screw it up if we tried to recreate their rules. http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/util/net/net_util.h File src/kudu/util/net/net_util.h: http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/util/net/net_util.h@133 PS2, Line 133: bool ValidateAddressListFlag(const char* flag_name, const std::string& addr_list); > warning: function 'kudu::ValidateAddressListFlag' has a definition with dif Done -- To view, visit http://gerrit.cloudera.org:8080/8312 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1b8f360408b05d6ac4f7359a8ac0dd889b196baf Gerrit-Change-Number: 8312 Gerrit-PatchSet: 3 Gerrit-Owner: Dan Burkert <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Mon, 13 Nov 2017 22:54:33 +0000 Gerrit-HasComments: Yes
