Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12919 )

Change subject: sentry: sanitize and parse privileges from Sentry
......................................................................


Patch Set 4: Code-Review+1

(3 comments)

Looks good to me, but it seems one test is failing?

http://gerrit.cloudera.org:8080/#/c/12919/3/src/kudu/master/sentry_authz_provider.cc
File src/kudu/master/sentry_authz_provider.cc:

http://gerrit.cloudera.org:8080/#/c/12919/3/src/kudu/master/sentry_authz_provider.cc@325
PS3, Line 325:  InsertOrDie(&expected_empt
> It sounds more like an English sentence as is, i.e. if (SentryPrivilegeIsWe
Ah, OK.  SGTM.


http://gerrit.cloudera.org:8080/#/c/12919/3/src/kudu/sentry/sentry_action.h
File src/kudu/sentry/sentry_action.h:

http://gerrit.cloudera.org:8080/#/c/12919/3/src/kudu/sentry/sentry_action.h@53
PS3, Line 53:    ALL,
> I somewhat see the point of this, but don't feel strongly enough about it t
That was just a nit, yep.  Most likely I just missed this nit when 
sentry_action.h just appeared, and I agree there are multiple criteria on the 
ordering :)

Keeping it as is sounds good to me.


http://gerrit.cloudera.org:8080/#/c/12919/3/src/kudu/sentry/sentry_authorizable_scope.h
File src/kudu/sentry/sentry_authorizable_scope.h:

http://gerrit.cloudera.org:8080/#/c/12919/3/src/kudu/sentry/sentry_authorizable_scope.h@91
PS3, Line 91:
> >Is it possible to create the result AuthorizableScopesSet once and the jus
Ah, I didn't explain the idea clear enough, sorry.

The idea was to have static instances of AuthorizableScopesSet per scope in 
body of this function, and just return const reference to the corresponding 
instance.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6de6814f99abfbee4f030298b74f21f4e7c729b
Gerrit-Change-Number: 12919
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 04 Apr 2019 17:12:28 +0000
Gerrit-HasComments: Yes

Reply via email to