> On Sept. 24, 2018, 11:23 p.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
> > Lines 915 (patched)
> > <https://reviews.apache.org/r/68822/diff/2/?file=2091812#file2091812line915>
> >
> >     you have got groups in line 908 if the requestor is the same as 
> > principlaName. It should improve performance to reuse it under this 
> > situation

I don't think the requestor is the same as principal? Here it is getting the 
principal's group.


> On Sept. 24, 2018, 11:23 p.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java
> > Line 227 (original), 229 (patched)
> > <https://reviews.apache.org/r/68822/diff/2/?file=2091814#file2091814line229>
> >
> >     This is semi-public API. It is safer to add function than change the 
> > returned type of existing function. Inside implementation, you can let one 
> > function call another one to re-use code.

I was hesitate to add another API to complicate the code base. Also I do not 
see any other consumer of this API other than SentryPolicyStoreProcessor so I 
made change on top of the existing one. But if you prefer to add a new one to 
avoid unknown dependency, I will update this.


- Hao


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68822/#review208969
-----------------------------------------------------------


On Sept. 24, 2018, 9:31 p.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68822/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2018, 9:31 p.m.)
> 
> 
> Review request for sentry, Na Li and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This commit adds a new thrift API list_sentry_privileges_for_user to List 
> sentry privileges granted to the given user and the group the user associated 
> with, filterted based on authorization hierarchy if present.
> Under the hood, this API is using sentryStore.listSentryPrivilegesForProvider.
> 
> 
> Diffs
> -----
> 
>   
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/SentryPolicyService.java
>  0cbd8ab0a624d4c09aead4097f72762e12d1d21b 
>   
> sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift
>  2e79e5646ae9102d8c0c28da4260a539254fcd15 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
>  236a07bdf5191cdc0f167f20a406b721b3dc506d 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
>  3a9623b46f7c4335db18113574170f761da9a4ca 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  1eda41b2b6bd940a404cc1ba09a861fe783ead04 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java
>  0b4f4aa24bd3002c50bf4d80a6fa361f66052973 
>   
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestTSentryPrivilegeToAuthorizable.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  d8c0ab4fa82ba09c60bc995eb4f53a78a7fae346 
>   
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStoreToAuthorizable.java
>  25f94fa05b05abf8c1dbc33e97e5e88ae01794e4 
> 
> 
> Diff: https://reviews.apache.org/r/68822/diff/2/
> 
> 
> Testing
> -------
> 
> Unit test.
> 
> 
> Thanks,
> 
> Hao Hao
> 
>

Reply via email to