Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13069 )

Change subject: [authz] new SentryAuthzProvider's caching strategy
......................................................................


Patch Set 6: Code-Review+1

(5 comments)

LGTM, just a few nits.

http://gerrit.cloudera.org:8080/#/c/13069/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13069/5//COMMIT_MSG@87
PS5, Line 87: irrelevant information on non-Kudu
            : tables sent by Sentry with a _database-scope_ response
Might worth a test for this as well?


http://gerrit.cloudera.org:8080/#/c/13069/6/src/kudu/master/sentry_authz_provider-test.cc
File src/kudu/master/sentry_authz_provider-test.cc:

http://gerrit.cloudera.org:8080/#/c/13069/6/src/kudu/master/sentry_authz_provider-test.cc@776
PS6, Line 776: db
nit: still a bit confused here, do you mean 'db' in L780? Because you are doing 
AuthorizeCreateTable for 'db1' there but the comment is showing otherwise.


http://gerrit.cloudera.org:8080/#/c/13069/6/src/kudu/master/sentry_privileges_fetcher.cc
File src/kudu/master/sentry_privileges_fetcher.cc:

http://gerrit.cloudera.org:8080/#/c/13069/6/src/kudu/master/sentry_privileges_fetcher.cc@225
PS6, Line 225: GetFlattenedKey
nit: can you add a comment here?


http://gerrit.cloudera.org:8080/#/c/13069/6/src/kudu/master/sentry_privileges_fetcher.cc@234
PS6, Line 234: 2
nit: can you briefly explain why it is 2 now?


http://gerrit.cloudera.org:8080/#/c/13069/6/src/kudu/master/sentry_privileges_fetcher.cc@529
PS6, Line 529: uncomment when it's guaranteed we are not getting
             :   //                requests for authorizables of scope wider 
than DATABASE.
We don't have any Server level authorizables request today? Or there are some 
in tests?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id96181345e357a104e28314d8d8d88633dcf9608
Gerrit-Change-Number: 13069
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <hao....@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 23 Apr 2019 19:17:00 +0000
Gerrit-HasComments: Yes

Reply via email to