> On 一月 28, 2015, 7:13 a.m., Xiaomeng Huang wrote:
> > sentry-binding/sentry-binding-solr/pom.xml, line 56
> > <https://reviews.apache.org/r/29240/diff/3/?file=816335#file816335line56>
> >
> >     ?

The tool needs the class SimpleFileProviderBackend. This class in the 
sentry-provider-file package. So the test scope was needed to delete


> On 一月 28, 2015, 7:13 a.m., Xiaomeng Huang wrote:
> > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrSentryConfigTool.java,
> >  line 151
> > <https://reviews.apache.org/r/29240/diff/3/?file=816336#file816336line151>
> >
> >     Could these System.out.println be log?

Because the tool is a command utility, it communicates with console.using 
System.out.println is better than the log4j


> On 一月 28, 2015, 7:13 a.m., Xiaomeng Huang wrote:
> > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrSentryConfigTool.java,
> >  line 161
> > <https://reviews.apache.org/r/29240/diff/3/?file=816336#file816336line161>
> >
> >     the requestorName is solid? So any user can execute this public API 
> > even if it is not admin?

This is not the problem. The sentry service side using the GSSCallback to allow 
the sentry.service.allow.connect users to connect.


> On 一月 28, 2015, 7:13 a.m., Xiaomeng Huang wrote:
> > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrSentryConfigTool.java,
> >  line 197
> > <https://reviews.apache.org/r/29240/diff/3/?file=816336#file816336line197>
> >
> >     could you rename a better name for "a"

Thanks xiaomeng. I will fix it


> On 一月 28, 2015, 7:13 a.m., Xiaomeng Huang wrote:
> > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrSentryConfigTool.java,
> >  line 274
> > <https://reviews.apache.org/r/29240/diff/3/?file=816336#file816336line274>
> >
> >     switch.. case?

It is using the same mechanism as hive tool.


> On 一月 28, 2015, 7:13 a.m., Xiaomeng Huang wrote:
> > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/binding/solr/TestSolrSentryConfigTool.java,
> >  line 57
> > <https://reviews.apache.org/r/29240/diff/3/?file=816337#file816337line57>
> >
> >     Could this be BeforeClass? so that we will cutdown test time if there 
> > are many test cases.

Thanks xiaomeng. A good suggestion. I will fix it.


> On 一月 28, 2015, 7:13 a.m., Xiaomeng Huang wrote:
> > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrSentryConfigTool.java,
> >  line 331
> > <https://reviews.apache.org/r/29240/diff/3/?file=816336#file816336line331>
> >
> >     why we need write a KeyValue class? map is not enough? or other java 
> > container?

I will delete the KeyValue class. These already exists a KeyValue in the source 
code. Thanks for your comment


- shen


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


On 一月 12, 2015, 9:12 a.m., shen guoquan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29240/
> -----------------------------------------------------------
> 
> (Updated 一月 12, 2015, 9:12 a.m.)
> 
> 
> Review request for sentry, 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:
> 
> Can't satisfied with the needs of dynamically add, delete and update 
> permissions
> 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-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrSentryConfigTool.java
>  PRE-CREATION 
>   
> sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/binding/solr/TestSolrSentryConfigTool.java
>  PRE-CREATION 
>   
> sentry-binding/sentry-binding-solr/src/test/resources/solr_config_import_tool.ini
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29240/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> shen guoquan
> 
>

Reply via email to