> On 一月 28, 2015, 3:58 a.m., Dapeng Sun wrote:
> > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/sentry/SentryIndexAuthorizationSingleton.java,
> >  line 55
> > <https://reviews.apache.org/r/30068/diff/1/?file=826505#file826505line55>
> >
> >     tmpClient may be unnecessary

Thanks Dapeng. I will fix it.


> On 一月 28, 2015, 3:58 a.m., Dapeng Sun wrote:
> > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/sentry/SentryIndexAuthorizationSingleton.java,
> >  line 89
> > <https://reviews.apache.org/r/30068/diff/1/?file=826505#file826505line89>
> >
> >     Why not use **tmpBinding.useSentryService()**

Good idea. I will fix it. Thanks Dapeng


> On 一月 28, 2015, 3:58 a.m., Dapeng Sun wrote:
> > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/TestSolrSyncPrivilegeOperation.java,
> >  line 66
> > <https://reviews.apache.org/r/30068/diff/1/?file=826506#file826506line66>
> >
> >     SearchConstants.ALL means action all? Is there any other actions need 
> > to be tested?

SearchConstants.ALL means action all. I will add other actions to test. Thanks 
dapeng.


> On 一月 28, 2015, 3:58 a.m., Dapeng Sun wrote:
> > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/sentry/SentryIndexAuthorizationSingleton.java,
> >  line 93
> > <https://reviews.apache.org/r/30068/diff/1/?file=826505#file826505line93>
> >
> >     the Method is unnecessary, we should throw the exception, it will 
> > handle by **SentryIndexAuthorizationSingleton**

I agree with you. Thanks for your comments. I will fix it.


> On 一月 28, 2015, 3:58 a.m., Dapeng Sun wrote:
> > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/sentry/SentryIndexAuthorizationSingleton.java,
> >  line 229
> > <https://reviews.apache.org/r/30068/diff/1/?file=826505#file826505line229>
> >
> >     If a user can delete the conllection, would he have the revoke 
> > permission?

Thanks, dapeng. I will fix it.


- shen


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


On 一月 20, 2015, 11:01 a.m., shen guoquan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30068/
> -----------------------------------------------------------
> 
> (Updated 一月 20, 2015, 11:01 a.m.)
> 
> 
> Review request for sentry, Gregory Chanan and Vamsee Yarlagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Solr needs to sync collection privileges with Sentry when the metadata has 
> changed. For example, when the collection has been deleted, the privileges 
> related to the collection were needed to drop
> 
> 
> Diffs
> -----
> 
>   
> sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java
>  faf862f 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java
>  d6600a0 
>   
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/SecureRequestHandlerUtil.java
>  8b46388 
>   
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/admin/SecureCollectionsHandler.java
>  20d9626 
>   
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/sentry/SentryIndexAuthorizationSingleton.java
>  d44b228 
>   
> sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/TestSolrSyncPrivilegeOperation.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/30068/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> shen guoquan
> 
>

Reply via email to