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


Fix it, then Ship it!




LGTM otherwise.


sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryConfigToolSolr.java
 (lines 46 - 48)
<https://reviews.apache.org/r/46058/#comment192201>

    Can't we put these files under test/resources?


- Vamsee Yarlagadda


On April 12, 2016, 12:38 a.m., Gregory Chanan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46058/
> -----------------------------------------------------------
> 
> (Updated April 12, 2016, 12:38 a.m.)
> 
> 
> Review request for sentry, shen guoquan, Sravya Tirukkovalur, and Vamsee 
> Yarlagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Updates guoquan's original patch to the latest Sentry API's and layout.
> 
> In particular the differences from the original patch are:
> 1) Convert to the latest Generic APIs
> 2) Is a tool like the SolrSentryShell supporting similar flags
> 3) No longer builds a (SolrAuthz)Binding, only uses the file provider backend 
> and the validators. Thus, we don't require the binding which is tricky to get 
> right anyway (the binding really runs in the server...are we convinced we 
> have all the right system properties, etc to get it to work)?
> 4) because of 3), doesn't pass the policy file via the configuration file, 
> takes it on the command line. We need the configuration file to specify the 
> service info; having both the policy file and the service info in there means 
> users would have to create a configuration file just for use of the tool.
> 5) Adds some additional compatibility checking around the fact that role 
> names are lower cased in the service but at least the solr backend supported 
> cased role names.
> 
> Like the SolrSentryShell, this should probably live in sole solr-specific 
> client module, but since that doesn't exist yet, I put it in the 
> sentry-provider-db, which is the same place the SolrSentryShell exists.
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/pom.xml 
> b6efd1f2b7f2178b46b8538858292d31ea60743e 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolCommon.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SolrTSentryPrivilegeConvertor.java
>  e2dfdf13d0c5f665080fb1c35da8b1cb648019df 
>   sentry-provider/sentry-provider-db/src/main/resources/solr_case.ini 
> PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/resources/solr_config_import_tool.ini
>  PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/resources/solr_invalid.ini 
> PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryConfigToolSolr.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46058/diff/
> 
> 
> Testing
> -------
> 
> Ran the unit tests successfully.
> 
> 
> Thanks,
> 
> Gregory Chanan
> 
>

Reply via email to