You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Lenni Kuff <ls...@cloudera.com> on 2015/02/07 02:40:07 UTC
Re: Review Request 29238: SENTRY-479:Create a Solr client to
interactive with the sentry service
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29238/#review71535
-----------------------------------------------------------
Ship it!
sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchConstants.java
<https://reviews.apache.org/r/29238/#comment117276>
used to distinguish...
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationComponent.java
<https://reviews.apache.org/r/29238/#comment117277>
Grammar. Maybe "Component being authorized by Sentry"?
Also, HIVE is very specific (it excludes Impala). Perhaps "SQL" or "DB"
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SearchPolicyServiceClient.java
<https://reviews.apache.org/r/29238/#comment117278>
Why do you have an enum for AuthorizationComponent when you are using it like a String? Either change AithorizationComponent to be string constants or make this an enum.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SearchProviderBackend.java
<https://reviews.apache.org/r/29238/#comment117279>
Add class comment.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SearchProviderBackend.java
<https://reviews.apache.org/r/29238/#comment117280>
Would it make this easier to use if you could call initialize multiple times?
- Lenni Kuff
On Jan. 30, 2015, 5:55 a.m., shen guoquan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29238/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2015, 5:55 a.m.)
>
>
> Review request for sentry, Xiaomeng Huang, Colin Ma, Dapeng Sun, Gregory Chanan, and Vamsee Yarlagadda.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Currently Solr does support index-level security via sentry with file as backend privilege store. Using file as the backend store is simple, but there has some disadvantages as followings:
> 1. Can't satisfied with the needs of dynamically add, delete and update permissions
> 2. Can't be centrally managed and difficult to maintain
> According to the above disadvantages, The Solr Sentry plug-in integration with DB store is demanded. The Hive Sentry plug-in has already integration with DB store, but the Hive authorization model is different from the Solr authorization model.So this new feature depends on the generic authorization model(SENTRY-398).
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-solr/pom.xml 2dfc933
> sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchConstants.java 7a39b79
> sentry-policy/sentry-policy-search/src/main/java/org/apache/sentry/policy/search/SimpleSearchPolicyEngine.java f428aea
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationComponent.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SearchPolicyServiceClient.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SearchProviderBackend.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/29238/diff/
>
>
> Testing
> -------
>
> All tests have passed through the local jenkins environment.
>
>
> Thanks,
>
> shen guoquan
>
>
Re: Review Request 29238: SENTRY-479:Create a Solr client to
interactive with the sentry service
Posted by shen guoquan <gu...@intel.com>.
> On 二月 7, 2015, 1:40 a.m., Lenni Kuff wrote:
> >
Hi Lenni. Thanks for your review. I will update the newset patch as soon as possible. Please help look at it. If you have no more comment, can you give the jira sentry-478 plus one. Thanks for your help
> On 二月 7, 2015, 1:40 a.m., Lenni Kuff wrote:
> > sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchConstants.java, line 25
> > <https://reviews.apache.org/r/29238/diff/6/?file=841302#file841302line25>
> >
> > used to distinguish...
Thanks Lenni. I will fix it.
> On 二月 7, 2015, 1:40 a.m., Lenni Kuff wrote:
> > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationComponent.java, line 19
> > <https://reviews.apache.org/r/29238/diff/6/?file=841304#file841304line19>
> >
> > Grammar. Maybe "Component being authorized by Sentry"?
> >
> > Also, HIVE is very specific (it excludes Impala). Perhaps "SQL" or "DB"
Thanks Lenni for your advice. I will fix it.
> On 二月 7, 2015, 1:40 a.m., Lenni Kuff wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SearchProviderBackend.java, line 38
> > <https://reviews.apache.org/r/29238/diff/6/?file=841306#file841306line38>
> >
> > Add class comment.
It's my mistake. I will fix it . Thanks for your comment.
> On 二月 7, 2015, 1:40 a.m., Lenni Kuff wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SearchProviderBackend.java, line 57
> > <https://reviews.apache.org/r/29238/diff/6/?file=841306#file841306line57>
> >
> > Would it make this easier to use if you could call initialize multiple times?
hi lenni. I think the initialize function can't be called multiple times. I use the same logic as the hive component backend provider. Thanks for your comment.
- shen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29238/#review71535
-----------------------------------------------------------
On 一月 30, 2015, 5:55 a.m., shen guoquan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29238/
> -----------------------------------------------------------
>
> (Updated 一月 30, 2015, 5:55 a.m.)
>
>
> Review request for sentry, Xiaomeng Huang, Colin Ma, Dapeng Sun, Gregory Chanan, and Vamsee Yarlagadda.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Currently Solr does support index-level security via sentry with file as backend privilege store. Using file as the backend store is simple, but there has some disadvantages as followings:
> 1. Can't satisfied with the needs of dynamically add, delete and update permissions
> 2. Can't be centrally managed and difficult to maintain
> According to the above disadvantages, The Solr Sentry plug-in integration with DB store is demanded. The Hive Sentry plug-in has already integration with DB store, but the Hive authorization model is different from the Solr authorization model.So this new feature depends on the generic authorization model(SENTRY-398).
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-solr/pom.xml 2dfc933
> sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchConstants.java 7a39b79
> sentry-policy/sentry-policy-search/src/main/java/org/apache/sentry/policy/search/SimpleSearchPolicyEngine.java f428aea
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationComponent.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SearchPolicyServiceClient.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SearchProviderBackend.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/29238/diff/
>
>
> Testing
> -------
>
> All tests have passed through the local jenkins environment.
>
>
> Thanks,
>
> shen guoquan
>
>