[kudu-CR] KUDU-2191 (4/n): WIP: Hive Metastore catalog manager integration

["Dan Burkert (Code Review)" <ge...@cloudera.org>]

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 <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 13 Nov 2017 22:54:33 +0000
Gerrit-HasComments: Yes