Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11524 )

Change subject: KUDU-2541: Fill out basic sentry client API
......................................................................


Patch Set 3:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/11524/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11524/1//COMMIT_MSG@9
PS1, Line 9: This commit contains more plumbing for the upcoming Sentry 
integration
> plumbing. Or plumbin'
Done


http://gerrit.cloudera.org:8080/#/c/11524/1//COMMIT_MSG@12
PS1, Line 12: Thrift'
> Thrift's
Done


http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/mini_sentry.cc
File src/kudu/sentry/mini_sentry.cc:

http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/mini_sentry.cc@142
PS1, Line 142: Hadoop'
> Hadoop's
Done


http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/mini_sentry.cc@147
PS1, Line 147: hav
> have
Done


http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/mini_sentry.cc@199
PS1, Line 199:   // Simple file format containing mapping of user to groups in 
INI syntax, see
> Maybe a short comment documenting the effect these values have?
Done


http://gerrit.cloudera.org:8080/#/c/11524/2/src/kudu/sentry/sentry_client-test.cc
File src/kudu/sentry/sentry_client-test.cc:

http://gerrit.cloudera.org:8080/#/c/11524/2/src/kudu/sentry/sentry_client-test.cc@53
PS2, Line 53: TEST_F(SentryClientTest, TestCreateDropRole) {
> Other simple tests for error conversion:
Done


http://gerrit.cloudera.org:8080/#/c/11524/2/src/kudu/sentry/sentry_client-test.cc@61
PS2, Line 61:     ::sentry::TCreateSentryRoleRequest req;
> How do you feel about building simple Kudu abstractions for the request/res
Right, the way this works on the HMS side is that we have the base hms-client 
which exposes the raw types, then a higher level component which provides a 
nicer API on top of that (hms-catalog).  I'm planning to do the same thing for 
Sentry.


http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/sentry_client.h
File src/kudu/sentry/sentry_client.h:

http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/sentry_client.h@60
PS1, Line 60:  thrift
> provided
Done


http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/sentry_client.h@61
PS1, Line 61:
> sentry
Done


http://gerrit.cloudera.org:8080/#/c/11524/1/src/kudu/sentry/sentry_client.h@61
PS1, Line 61:   ~SentryClient();
> warning: function 'kudu::sentry::SentryClient::SentryClient' has a definiti
Done



--
To view, visit http://gerrit.cloudera.org:8080/11524
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1004a11506ed109d1f23389ca73d95a34c32c194
Gerrit-Change-Number: 11524
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <andrew.w...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: Hao Hao <hao....@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 26 Sep 2018 22:25:50 +0000
Gerrit-HasComments: Yes

Reply via email to