Re: Review Request 69351: SENTRY-2458: Split web service from server service modules

[Steve Moist via Review Board]

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


Ship it!




Ship It!

- Steve Moist


On Nov. 27, 2018, 3:32 a.m., Brian Towles wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69351/
> ---
> 
> (Updated Nov. 27, 2018, 3:32 a.m.)
> 
> 
> Review request for sentry, Anthony Young-Garner, kalyan kumar kalvagadda, Na 
> Li, Steve Moist, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2458
> https://issues.apache.org/jira/browse/SENTRY-2458
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-2458: Split web service from server service modules
> 
> In order to additional modules to be added to Sentry there needs to be a 
> separation of some of the features used by service-server into other modules.
> 
> This will allow the web server to be pulled out from the base server and 
> allow for additional modules to be able to add web services and functionality 
> to the web interface without depending up the whole server module.
> 
> It will use the SPI to dynamically include servlets and content from other 
> modules.
> 
> It creates a sentry-service-providers to allow for Server and sub modules SPI 
> providers to be defined externally and not depended on directly.
> 
> 
> Diffs
> -
> 
>   pom.xml 46ca38e9a 
>   sentry-dist/pom.xml 62558d2e0 
>   sentry-dist/src/main/assembly/bin.xml 986530c55 
>   sentry-provider/sentry-provider-db/pom.xml df569474a 
>   sentry-service/pom.xml e653189eb 
>   sentry-service/sentry-service-providers/pom.xml PRE-CREATION 
>   
> sentry-service/sentry-service-providers/src/main/java/org/apache/sentry/server/provider/webservice/AttributeDesc.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-providers/src/main/java/org/apache/sentry/server/provider/webservice/FilterDesc.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-providers/src/main/java/org/apache/sentry/server/provider/webservice/ServletDesc.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-providers/src/main/java/org/apache/sentry/server/provider/webservice/WebServiceProvider.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-providers/src/main/java/org/apache/sentry/server/provider/webservice/WebServiceProviderFactory.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-providers/src/main/java/org/apache/sentry/server/provider/webservice/WebServiceSpi.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-providers/src/main/resources/META-INF/services/org.apache.sentry.spi.Spi
>  PRE-CREATION 
>   sentry-service/sentry-service-server/pom.xml 44540ad5d 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/ConfServlet.java
>  862548745 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/LogLevelServlet.java
>  af81d6fce 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/PubSubServlet.java
>  8da35f10f 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/RolesServlet.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryAdminServlet.java
>  5dc6cd6c4 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryAuthFilter.java
>  23121ecf5 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryServiceWebServiceProvider.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryWebServer.java
>  befe6c3ed 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  b3a4934df 
>   
> sentry-service/sentry-service-server/src/main/resources/META-INF/services/org.apache.sentry.server.provider.webservice.WebServiceProviderFactory
>  PRE-CREATION 
>   
> sentry-service/sentry-service-server/src/main/webapp/css/bootstrap-theme.min.css
>  c31428b07 
>   sentry-service/sentry-service-server/src/main/webapp/css/bootstrap.min.css 
> a553c4f5e 
>   sentry-service/sentry-service-server/src/main/webapp/css/sentry.css  
>   sentry-service/sentry-service-server/src/main/webapp/sentry.png  
>   
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryWebServerWithoutSecurity.java
>  6e741e895 
>   sentry-service/sentry-service-web/pom.xml PRE-CREATION 
>   
> sentry-service/sentry-service-web/src/main/java/org/apache/sentry/service/web/DefaultWebServicesProvider.java
>  PRE-CREATION 
>   
> 

Re: Review Request 69351: SENTRY-2458: Split web service from server service modules

[Steve Moist via Review Board]

> On Nov. 20, 2018, 6:04 p.m., Steve Moist wrote:
> > sentry-service/sentry-service-web/src/main/webapp/static/bootstrap/js/bootstrap.js
> > Lines 32 (patched)
> > 
> >
> > Why do we have 2 different copyrights?  Is this file under the MIT 
> > license?
> 
> Brian Towles wrote:
> This is a direct copy of the file.  It how they copyrighted it.

Even the apache 2 license on the top?


- Steve


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


On Nov. 20, 2018, 7:30 p.m., Brian Towles wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69351/
> ---
> 
> (Updated Nov. 20, 2018, 7:30 p.m.)
> 
> 
> Review request for sentry, Anthony Young-Garner, kalyan kumar kalvagadda, Na 
> Li, Steve Moist, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2458
> https://issues.apache.org/jira/browse/SENTRY-2458
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-2458: Split web service from server service modules
> 
> In order to additional modules to be added to Sentry there needs to be a 
> separation of some of the features used by service-server into other modules.
> 
> This will allow the web server to be pulled out from the base server and 
> allow for additional modules to be able to add web services and functionality 
> to the web interface without depending up the whole server module.
> 
> It will use the SPI to dynamically include servlets and content from other 
> modules.
> 
> It creates a sentry-service-providers to allow for Server and sub modules SPI 
> providers to be defined externally and not depended on directly.
> 
> 
> Diffs
> -
> 
>   pom.xml 46ca38e9a 
>   sentry-dist/pom.xml 62558d2e0 
>   sentry-dist/src/main/assembly/bin.xml 986530c55 
>   sentry-provider/sentry-provider-db/pom.xml df569474a 
>   sentry-service/pom.xml e653189eb 
>   sentry-service/sentry-service-providers/pom.xml PRE-CREATION 
>   
> sentry-service/sentry-service-providers/src/main/java/org/apache/sentry/server/provider/webservice/AttributeDesc.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-providers/src/main/java/org/apache/sentry/server/provider/webservice/FilterDesc.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-providers/src/main/java/org/apache/sentry/server/provider/webservice/SentryWebServiceProvider.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-providers/src/main/java/org/apache/sentry/server/provider/webservice/SentryWebServiceProviderFactory.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-providers/src/main/java/org/apache/sentry/server/provider/webservice/SentryWebServiceSpi.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-providers/src/main/java/org/apache/sentry/server/provider/webservice/ServletDesc.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-providers/src/main/resources/META-INF/services/org.apache.sentry.spi.Spi
>  PRE-CREATION 
>   sentry-service/sentry-service-server/pom.xml 44540ad5d 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/ConfServlet.java
>  862548745 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/LogLevelServlet.java
>  af81d6fce 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/PubSubServlet.java
>  8da35f10f 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/RolesServlet.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryAdminServlet.java
>  5dc6cd6c4 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryAuthFilter.java
>  23121ecf5 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryServiceWebServiceProvider.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryWebServer.java
>  befe6c3ed 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  b3a4934df 
>   
> sentry-service/sentry-service-server/src/main/resources/META-INF/services/org.apache.sentry.server.provider.webservice.SentryWebServiceProviderFactory
>  PRE-CREATION 
>   
> sentry-service/sentry-service-server/src/main/webapp/css/bootstrap-theme.min.css
>  c31428b07 
>   sentry-service/sentry-service-server/src/main/webapp/css/bootstrap.min.css 
> a553c4f5e 
>   sentry-service/sentry-service-server/src/main/webapp/css/sentry.css  
>   

Re: Review Request 69351: SENTRY-2458: Split web service from server service modules

[Steve Moist via Review Board]

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




sentry-dist/pom.xml
Lines 66 (patched)


You did an exclusion of * above, why not do it again here?



sentry-dist/src/main/assembly/bin.xml
Lines 49 (patched)


Why twice?



sentry-provider/sentry-provider-db/pom.xml
Lines 179 (patched)


Is there a differnence between the long list of exclusions, * or jetty*?  
Mainly asking for consistency's sake.



sentry-service/sentry-service-providers/pom.xml
Lines 33 (patched)


Add Name tag



sentry-service/sentry-service-server/pom.xml
Line 235 (original)


This was removed, is it still needed elsewhere?



sentry-service/sentry-service-web/src/main/java/org/apache/sentry/service/web/SentryWebServer.java
Lines 1 (patched)


This file has been moved, can we have git mark it as so instead of a new 
file.



sentry-service/sentry-service-web/src/main/webapp/static/bootstrap/css/bootstrap-theme.min.css
Lines 1 (patched)


Is it worth it to add the bootstrap version info to either the file name or 
in a comment?



sentry-service/sentry-service-web/src/main/webapp/static/bootstrap/css/bootstrap.min.css
Lines 1 (patched)


Same comment as above about bootstrap version.



sentry-service/sentry-service-web/src/main/webapp/static/bootstrap/js/bootstrap.js
Lines 1 (patched)


Version info again



sentry-service/sentry-service-web/src/main/webapp/static/bootstrap/js/bootstrap.js
Lines 32 (patched)


Why do we have 2 different copyrights?  Is this file under the MIT license?



sentry-service/sentry-service-web/src/main/webapp/static/bootstrap/js/bootstrap.js
Lines 92 (patched)


Why is there another one here?


- Steve Moist


On Nov. 15, 2018, 8:05 p.m., Brian Towles wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69351/
> ---
> 
> (Updated Nov. 15, 2018, 8:05 p.m.)
> 
> 
> Review request for sentry, Anthony Young-Garner, kalyan kumar kalvagadda, Na 
> Li, Steve Moist, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2458
> https://issues.apache.org/jira/browse/SENTRY-2458
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-2458: Split web service from server service modules
> 
> In order to additional modules to be added to Sentry there needs to be a 
> separation of some of the features used by service-server into other modules.
> 
> This will allow the web server to be pulled out from the base server and 
> allow for additional modules to be able to add web services and functionality 
> to the web interface without depending up the whole server module.
> 
> It will use the SPI to dynamically include servlets and content from other 
> modules.
> 
> It creates a sentry-service-providers to allow for Server and sub modules SPI 
> providers to be defined externally and not depended on directly.
> 
> 
> Diffs
> -
> 
>   pom.xml 46ca38e9a 
>   sentry-dist/pom.xml 62558d2e0 
>   sentry-dist/src/main/assembly/bin.xml 986530c55 
>   sentry-provider/sentry-provider-db/pom.xml df569474a 
>   sentry-service/pom.xml e653189eb 
>   sentry-service/sentry-service-providers/pom.xml PRE-CREATION 
>   
> sentry-service/sentry-service-providers/src/main/java/org/apache/sentry/server/provider/webservice/AttributeDesc.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-providers/src/main/java/org/apache/sentry/server/provider/webservice/FilterDesc.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-providers/src/main/java/org/apache/sentry/server/provider/webservice/SentryWebServiceProvider.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-providers/src/main/java/org/apache/sentry/server/provider/webservice/SentryWebServiceProviderFactory.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-providers/src/main/java/org/apache/sentry/server/provider/webservice/SentryWebServiceSpi.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-providers/src/main/java/org/apache/sentry/server/provider/webservice/ServletDesc.java
>  PRE-CREATION 
>   
> sentry-service/sentry-service-providers/src/main/resources/META-INF/services/org.apache.sentry.spi.Spi
>  

Re: Review Request 69285: Signal Handle Unregister

[Steve Moist via Review Board]

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


Ship it!




Ship It!

- Steve Moist


On Nov. 19, 2018, 4:32 p.m., Brian Towles wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69285/
> ---
> 
> (Updated Nov. 19, 2018, 4:32 p.m.)
> 
> 
> Review request for sentry, Anthony Young-Garner, kalyan kumar kalvagadda, Na 
> Li, Steve Moist, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2444
> https://issues.apache.org/jira/browse/SENTRY-2444
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-2444: Signal Handle Unregister
> 
> Single unregister of function from the signal handler.
> 
> 
> Diffs
> -
> 
>   
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SigUtils.java
>  d621c74c2 
> 
> 
> Diff: https://reviews.apache.org/r/69285/diff/2/
> 
> 
> Testing
> ---
> 
> Build and Unit tests
> 
> 
> Thanks,
> 
> Brian Towles
> 
>

Re: Review Request 69285: Signal Handle Unregister

[Steve Moist via Review Board]

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




sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SigUtils.java
Lines 120 (patched)


Reverse these checks to match the parameter order.



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SigUtils.java
Lines 123 (patched)


StringUtils.isEmpty


- Steve Moist


On Nov. 7, 2018, 9:57 p.m., Brian Towles wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69285/
> ---
> 
> (Updated Nov. 7, 2018, 9:57 p.m.)
> 
> 
> Review request for sentry, Anthony Young-Garner, kalyan kumar kalvagadda, Na 
> Li, Steve Moist, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2444
> https://issues.apache.org/jira/browse/SENTRY-2444
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-2444: Signal Handle Unregister
> 
> Single unregister of function from the signal handler.
> 
> 
> Diffs
> -
> 
>   
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SigUtils.java
>  d621c74c2cdf44c1eedbec36f5cd8a32cace20ff 
> 
> 
> Diff: https://reviews.apache.org/r/69285/diff/1/
> 
> 
> Testing
> ---
> 
> Build and Unit tests
> 
> 
> Thanks,
> 
> Brian Towles
> 
>

Re: Review Request 68573: SENTRY-2311 Intellij is broken by shaded jars

[Steve Moist via Review Board]

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

(Updated Sept. 7, 2018, 4:52 p.m.)


Review request for sentry.


Bugs: SENTRY-2311
https://issues.apache.org/jira/browse/SENTRY-2311


Repository: sentry


Description
---

Removed the shading from the individual projects and put it at the end.  This 
avoids Intellij issues and allows us to still shade the offending jars.  It 
then excludes the dependencies and adds them at the end.


Diffs (updated)
-

  pom.xml d5f9dc6f5 
  sentry-dist/pom.xml 6291e4f1c 
  sentry-dist/src/main/assembly/bin.xml fc012c671 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathDeltaRetriever.java
 0d39300fe 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java
 2b1618134 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermDeltaRetriever.java
 b9405ccd2 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java
 f3a2d5028 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java
 5e2d5c5ee 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsMetricsUtil.java
 f299825ba 
  sentry-provider/sentry-provider-db/pom.xml 9f89ca30a 
  sentry-service/sentry-service-server/pom.xml 8315358fc 
  sentry-thirdparty/pom.xml PRE-CREATION 
  sentry-thirdparty/sentry-shaded/pom.xml PRE-CREATION 


Diff: https://reviews.apache.org/r/68573/diff/2/

Changes: https://reviews.apache.org/r/68573/diff/1-2/


Testing
---

Unpacked the provided jars and ran it in a cluster.


Thanks,

Steve Moist

Re: Review Request 68657: SENTRY-2392: Add metrics statistics to list_user_privileges and list_role_privileges API

[Steve Moist via Review Board]

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




sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
Line 1397 (original), 1397 (patched)


Shouldn't there be a finally block here to stop the timer?


- Steve Moist


On Sept. 6, 2018, 2:57 p.m., Sergio Pena wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68657/
> ---
> 
> (Updated Sept. 6, 2018, 2:57 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
> 
> 
> Bugs: sentry-2392
> https://issues.apache.org/jira/browse/sentry-2392
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Add some metric statistics to the new SentryPolicyStoreProcessor API that 
> returns a list of users and roles privileges. This would be good to collect 
> in order to understand how this API is used.
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
>  35c7d079be3de8d0cf7e8067d4b5ef46c10f8d72 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
>  008a48efc0b1a2a8703a301c85cf0068dd171f38 
> 
> 
> Diff: https://reviews.apache.org/r/68657/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>

Re: Review Request 68647: SENTRY-2375: Fixed typos in listPrivilegesbyAuthorizable and getAllPrivilegesbyAuthorizable APIs

[Steve Moist via Review Board]

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


Ship it!




Ship It!

- Steve Moist


On Sept. 6, 2018, 5:36 a.m., Morio Ramdenbourg wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68647/
> ---
> 
> (Updated Sept. 6, 2018, 5:36 a.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Steve Moist, and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> [SENTRY-2375](https://issues.apache.org/jira/browse/SENTRY-2375): Fixed typos 
> in listPrivilegesbyAuthorizable and getAllPrivilegesbyAuthorizable APIs
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java
>  5db796dd45fcc026bfbbd17173b4c54d8051611e 
>   
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java
>  3ef1624e824881489fbf0798d7d1d0fa7688e5f0 
>   
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java
>  a2213ae35e62b15f649940ab24ff90cc87f0cd90 
>   
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryServiceIntegration.java
>  73fca848ba333aa4d7d7198f82c5bd03c09c0791 
> 
> 
> Diff: https://reviews.apache.org/r/68647/diff/1/
> 
> 
> Testing
> ---
> 
> Ran the test phase (mvn clean install) from the root level successfully.
> 
> 
> Thanks,
> 
> Morio Ramdenbourg
> 
>

Re: Review Request 68488: SENTRY-2367: Implement subsystem to allow for pluggable attribute providers and transports

[Steve Moist via Review Board]

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


Ship it!




Ship It!

- Steve Moist


On Aug. 31, 2018, 4:03 p.m., Brian Towles wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68488/
> ---
> 
> (Updated Aug. 31, 2018, 4:03 p.m.)
> 
> 
> Review request for sentry, Anthony Young-Garner, kalyan kumar kalvagadda, Na 
> Li, Steve Moist, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2367
> https://issues.apache.org/jira/browse/SENTRY-2367
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Basic implementation of the Java SPI. Abstracted to provide Factory 
> configuration for
> instances of Service defined.
> 
> Currently stand alone module.  Will be built used by MDCM in future commits.
> 
> 
> Diffs
> -
> 
>   pom.xml fb9950d0feca18f7f49531c6c38bb7357b565e5b 
>   sentry-spi/pom.xml PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/DefaultProviderLoader.java 
> PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/Provider.java PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/ProviderFactory.java 
> PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/ProviderLoader.java 
> PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/ProviderManager.java 
> PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/Spi.java PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/package-info.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProvider.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProviderFactory.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProviderImplA.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProviderImplB.java 
> PRE-CREATION 
>   
> sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProviderImplNotLoaded.java
>  PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestSpi.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/TestProviderManager.java 
> PRE-CREATION 
>   
> sentry-spi/src/test/resources/META-INF/services/org.apache.sentry.spi.SomeTestProviderFactory
>  PRE-CREATION 
>   sentry-spi/src/test/resources/META-INF/services/org.apache.sentry.spi.Spi 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68488/diff/3/
> 
> 
> Testing
> ---
> 
> Unit tests.
> 
> 
> Thanks,
> 
> Brian Towles
> 
>

Re: Review Request 68488: SENTRY-2367: Implement subsystem to allow for pluggable attribute providers and transports

[Steve Moist via Review Board]

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




sentry-spi/pom.xml
Lines 45 (patched)


Sorry if I wasnt' clear.  For SENTRY-2374 add all of the lombock changes 
into there and use the existing loggers for now.


- Steve Moist


On Aug. 31, 2018, 4:03 p.m., Brian Towles wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68488/
> ---
> 
> (Updated Aug. 31, 2018, 4:03 p.m.)
> 
> 
> Review request for sentry, Anthony Young-Garner, kalyan kumar kalvagadda, Na 
> Li, Steve Moist, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2367
> https://issues.apache.org/jira/browse/SENTRY-2367
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Basic implementation of the Java SPI. Abstracted to provide Factory 
> configuration for
> instances of Service defined.
> 
> Currently stand alone module.  Will be built used by MDCM in future commits.
> 
> 
> Diffs
> -
> 
>   pom.xml fb9950d0feca18f7f49531c6c38bb7357b565e5b 
>   sentry-spi/pom.xml PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/DefaultProviderLoader.java 
> PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/Provider.java PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/ProviderFactory.java 
> PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/ProviderLoader.java 
> PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/ProviderManager.java 
> PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/Spi.java PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/package-info.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProvider.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProviderFactory.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProviderImplA.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProviderImplB.java 
> PRE-CREATION 
>   
> sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProviderImplNotLoaded.java
>  PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestSpi.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/TestProviderManager.java 
> PRE-CREATION 
>   
> sentry-spi/src/test/resources/META-INF/services/org.apache.sentry.spi.SomeTestProviderFactory
>  PRE-CREATION 
>   sentry-spi/src/test/resources/META-INF/services/org.apache.sentry.spi.Spi 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68488/diff/3/
> 
> 
> Testing
> ---
> 
> Unit tests.
> 
> 
> Thanks,
> 
> Brian Towles
> 
>

Re: Review Request 68488: SENTRY-2367: Implement subsystem to allow for pluggable attribute providers and transports

[Steve Moist via Review Board]

> On Aug. 27, 2018, 8:32 p.m., Steve Moist wrote:
> > pom.xml
> > Lines 410 (patched)
> > 
> >
> > Why is this provided?
> 
> Brian Towles wrote:
> Lombok only hooks on the compile and modified the output classes 
> directly.  You dont need to actually include it in the distrobution.
> 
> Steve Moist wrote:
> Should it be scope compile then?
> 
> Brian Towles wrote:
> No Compile scope includes it in the transative dependencies.  Provided 
> make it available for compile but does not include in transative dependencies.

Cool, I'm good on this change then.


- Steve


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


On Aug. 31, 2018, 4:03 p.m., Brian Towles wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68488/
> ---
> 
> (Updated Aug. 31, 2018, 4:03 p.m.)
> 
> 
> Review request for sentry, Anthony Young-Garner, kalyan kumar kalvagadda, Na 
> Li, Steve Moist, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2367
> https://issues.apache.org/jira/browse/SENTRY-2367
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Basic implementation of the Java SPI. Abstracted to provide Factory 
> configuration for
> instances of Service defined.
> 
> Currently stand alone module.  Will be built used by MDCM in future commits.
> 
> 
> Diffs
> -
> 
>   pom.xml fb9950d0feca18f7f49531c6c38bb7357b565e5b 
>   sentry-spi/pom.xml PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/DefaultProviderLoader.java 
> PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/Provider.java PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/ProviderFactory.java 
> PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/ProviderLoader.java 
> PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/ProviderManager.java 
> PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/Spi.java PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/package-info.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProvider.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProviderFactory.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProviderImplA.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProviderImplB.java 
> PRE-CREATION 
>   
> sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProviderImplNotLoaded.java
>  PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestSpi.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/TestProviderManager.java 
> PRE-CREATION 
>   
> sentry-spi/src/test/resources/META-INF/services/org.apache.sentry.spi.SomeTestProviderFactory
>  PRE-CREATION 
>   sentry-spi/src/test/resources/META-INF/services/org.apache.sentry.spi.Spi 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68488/diff/3/
> 
> 
> Testing
> ---
> 
> Unit tests.
> 
> 
> Thanks,
> 
> Brian Towles
> 
>

Re: Review Request 68573: SENTRY-2311 Intellij is broken by shaded jars

[Steve Moist via Review Board]

> On Aug. 30, 2018, 8:51 p.m., Na Li wrote:
> > have you tested that Intellij and CLI works fine with this change?

Yes.


- Steve


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


On Aug. 30, 2018, 7:37 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68573/
> ---
> 
> (Updated Aug. 30, 2018, 7:37 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-2311
> https://issues.apache.org/jira/browse/SENTRY-2311
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Removed the shading from the individual projects and put it at the end.  This 
> avoids Intellij issues and allows us to still shade the offending jars.  It 
> then excludes the dependencies and adds them at the end.
> 
> 
> Diffs
> -
> 
>   pom.xml fb9950d0f 
>   sentry-dist/pom.xml 6291e4f1c 
>   sentry-dist/src/license/THIRD-PARTY.properties a1084db69 
>   sentry-dist/src/main/assembly/bin.xml fc012c671 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathDeltaRetriever.java
>  0d39300fe 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java
>  2b1618134 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermDeltaRetriever.java
>  b9405ccd2 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java
>  f3a2d5028 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java
>  5e2d5c5ee 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsMetricsUtil.java
>  f299825ba 
>   sentry-provider/sentry-provider-db/pom.xml 9f89ca30a 
>   sentry-service/sentry-service-server/pom.xml 8315358fc 
>   sentry-thirdparty/pom.xml PRE-CREATION 
>   sentry-thirdparty/sentry-shaded/pom.xml PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68573/diff/1/
> 
> 
> Testing
> ---
> 
> Unpacked the provided jars and ran it in a cluster.
> 
> 
> Thanks,
> 
> Steve Moist
> 
>

Re: Review Request 68573: SENTRY-2311 Intellij is broken by shaded jars

[Steve Moist via Review Board]

> On Aug. 30, 2018, 8:50 p.m., Na Li wrote:
> > sentry-dist/src/license/THIRD-PARTY.properties
> > Lines 16 (patched)
> > 
> >
> > do we need to change this file? no real change

No I'll remove it.


- Steve


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


On Aug. 30, 2018, 7:37 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68573/
> ---
> 
> (Updated Aug. 30, 2018, 7:37 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-2311
> https://issues.apache.org/jira/browse/SENTRY-2311
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Removed the shading from the individual projects and put it at the end.  This 
> avoids Intellij issues and allows us to still shade the offending jars.  It 
> then excludes the dependencies and adds them at the end.
> 
> 
> Diffs
> -
> 
>   pom.xml fb9950d0f 
>   sentry-dist/pom.xml 6291e4f1c 
>   sentry-dist/src/license/THIRD-PARTY.properties a1084db69 
>   sentry-dist/src/main/assembly/bin.xml fc012c671 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathDeltaRetriever.java
>  0d39300fe 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java
>  2b1618134 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermDeltaRetriever.java
>  b9405ccd2 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java
>  f3a2d5028 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java
>  5e2d5c5ee 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsMetricsUtil.java
>  f299825ba 
>   sentry-provider/sentry-provider-db/pom.xml 9f89ca30a 
>   sentry-service/sentry-service-server/pom.xml 8315358fc 
>   sentry-thirdparty/pom.xml PRE-CREATION 
>   sentry-thirdparty/sentry-shaded/pom.xml PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68573/diff/1/
> 
> 
> Testing
> ---
> 
> Unpacked the provided jars and ran it in a cluster.
> 
> 
> Thanks,
> 
> Steve Moist
> 
>

Re: Review Request 68488: SENTRY-2367: Implement subsystem to allow for pluggable attribute providers and transports

[Steve Moist via Review Board]

> On Aug. 27, 2018, 8:32 p.m., Steve Moist wrote:
> > pom.xml
> > Lines 410 (patched)
> > 
> >
> > Why is this provided?
> 
> Brian Towles wrote:
> Lombok only hooks on the compile and modified the output classes 
> directly.  You dont need to actually include it in the distrobution.

Should it be scope compile then?


- Steve


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


On Aug. 24, 2018, 1:15 a.m., Brian Towles wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68488/
> ---
> 
> (Updated Aug. 24, 2018, 1:15 a.m.)
> 
> 
> Review request for sentry, Anthony Young-Garner, kalyan kumar kalvagadda, Na 
> Li, Steve Moist, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2367
> https://issues.apache.org/jira/browse/SENTRY-2367
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Basic implementation of the Java SPI. Abstracted to provide Factory 
> configuration for
> instances of Service defined.
> 
> Currently stand alone module.  Will be built used by MDCM in future commits.
> 
> 
> Diffs
> -
> 
>   lombok.config PRE-CREATION 
>   pom.xml dcf107680c3b395819043136bcc6e179a60ada35 
>   sentry-spi/pom.xml PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/DefaultProviderLoader.java 
> PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/Provider.java PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/ProviderFactory.java 
> PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/ProviderLoader.java 
> PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/ProviderManager.java 
> PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/Spi.java PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/package-info.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProvider.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProviderFactory.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProviderImplA.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProviderImplB.java 
> PRE-CREATION 
>   
> sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProviderImplNotLoaded.java
>  PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestSpi.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/TestProviderManager.java 
> PRE-CREATION 
>   
> sentry-spi/src/test/resources/META-INF/services/org.apache.sentry.spi.SomeTestProviderFactory
>  PRE-CREATION 
>   sentry-spi/src/test/resources/META-INF/services/org.apache.sentry.spi.Spi 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68488/diff/2/
> 
> 
> Testing
> ---
> 
> Unit tests.
> 
> 
> Thanks,
> 
> Brian Towles
> 
>

Review Request 68573: SENTRY-2311 Intellij is broken by shaded jars

[Steve Moist via Review Board]

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

Review request for sentry.


Bugs: SENTRY-2311
https://issues.apache.org/jira/browse/SENTRY-2311


Repository: sentry


Description
---

Removed the shading from the individual projects and put it at the end.  This 
avoids Intellij issues and allows us to still shade the offending jars.  It 
then excludes the dependencies and adds them at the end.


Diffs
-

  pom.xml fb9950d0f 
  sentry-dist/pom.xml 6291e4f1c 
  sentry-dist/src/license/THIRD-PARTY.properties a1084db69 
  sentry-dist/src/main/assembly/bin.xml fc012c671 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathDeltaRetriever.java
 0d39300fe 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java
 2b1618134 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermDeltaRetriever.java
 b9405ccd2 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java
 f3a2d5028 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java
 5e2d5c5ee 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsMetricsUtil.java
 f299825ba 
  sentry-provider/sentry-provider-db/pom.xml 9f89ca30a 
  sentry-service/sentry-service-server/pom.xml 8315358fc 
  sentry-thirdparty/pom.xml PRE-CREATION 
  sentry-thirdparty/sentry-shaded/pom.xml PRE-CREATION 


Diff: https://reviews.apache.org/r/68573/diff/1/


Testing
---

Unpacked the provided jars and ran it in a cluster.


Thanks,

Steve Moist

Re: Review Request 68488: SENTRY-2367: Implement subsystem to allow for pluggable attribute providers and transports

[Steve Moist via Review Board]

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




pom.xml
Lines 410 (patched)


Why is this provided?



sentry-spi/src/main/java/org/apache/sentry/spi/Spi.java
Lines 23 (patched)


Add a class description



sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProviderFactory.java
Lines 23 (patched)


Add to comment or remove.



sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProviderImplA.java
Lines 23 (patched)


Add to comment or remove.



sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProviderImplB.java
Lines 23 (patched)


Add to comment or remove.



sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProviderImplNotLoaded.java
Lines 23 (patched)


Add to comment or remove.


- Steve Moist


On Aug. 24, 2018, 1:15 a.m., Brian Towles wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68488/
> ---
> 
> (Updated Aug. 24, 2018, 1:15 a.m.)
> 
> 
> Review request for sentry, Anthony Young-Garner, kalyan kumar kalvagadda, Na 
> Li, Steve Moist, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2367
> https://issues.apache.org/jira/browse/SENTRY-2367
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Basic implementation of the Java SPI. Abstracted to provide Factory 
> configuration for
> instances of Service defined.
> 
> Currently stand alone module.  Will be built used by MDCM in future commits.
> 
> 
> Diffs
> -
> 
>   lombok.config PRE-CREATION 
>   pom.xml dcf107680c3b395819043136bcc6e179a60ada35 
>   sentry-spi/pom.xml PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/DefaultProviderLoader.java 
> PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/Provider.java PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/ProviderFactory.java 
> PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/ProviderLoader.java 
> PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/ProviderManager.java 
> PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/Spi.java PRE-CREATION 
>   sentry-spi/src/main/java/org/apache/sentry/spi/package-info.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProvider.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProviderFactory.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProviderImplA.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProviderImplB.java 
> PRE-CREATION 
>   
> sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestProviderImplNotLoaded.java
>  PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/SomeTestSpi.java 
> PRE-CREATION 
>   sentry-spi/src/test/java/org/apache/sentry/spi/TestProviderManager.java 
> PRE-CREATION 
>   
> sentry-spi/src/test/resources/META-INF/services/org.apache.sentry.spi.SomeTestProviderFactory
>  PRE-CREATION 
>   sentry-spi/src/test/resources/META-INF/services/org.apache.sentry.spi.Spi 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68488/diff/2/
> 
> 
> Testing
> ---
> 
> Unit tests.
> 
> 
> Thanks,
> 
> Brian Towles
> 
>

Review Request 68113: SENTRY-2230 Change sentry-service-server to use ${project.version}

[Steve Moist via Review Board]

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

Review request for sentry.


Bugs: SENTRY-2230
https://issues.apache.org/jira/browse/SENTRY-2230


Repository: sentry


Description
---

modified pom to use correct variable.


Diffs
-

  sentry-service/sentry-service-api/pom.xml 
a64a7e7ccb7308255a046671d20b46e43190c25a 
  sentry-service/sentry-service-server/pom.xml 
b95344e56545ffcf4414093f48e364f5d735c1cc 


Diff: https://reviews.apache.org/r/68113/diff/1/


Testing
---

mvn clean install


Thanks,

Steve Moist

Re: Review Request 67846: SENTRY-2283 Multiple versions of metrics on the classpath causes Sentry to not startup

[Steve Moist via Review Board]

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

(Updated July 9, 2018, 7:28 p.m.)


Review request for sentry.


Repository: sentry


Description
---

Excluding conflicting dependencies and shading io.dropwizard.metrics.


Diffs
-

  pom.xml 488426593 
  sentry-binding/sentry-binding-kafka/pom.xml 239eeba5f 
  sentry-dist/src/license/THIRD-PARTY.properties b39e1b6ca 
  sentry-dist/src/main/assembly/bin.xml 72773df1e 
  sentry-hdfs/sentry-hdfs-dist/pom.xml 93943157d 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathDeltaRetriever.java
 81c614a34 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java
 3532ef33d 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermDeltaRetriever.java
 8d6713acd 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java
 b87d29040 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java
 0cd405b54 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsMetricsUtil.java
 3bf300bef 
  sentry-provider/sentry-provider-db/pom.xml a8a15bfb1 
  sentry-service/sentry-service-server/pom.xml a103c1e25 
  sentry-tests/sentry-tests-solr/pom.xml db33ee9f4 
  sentry-tests/sentry-tests-sqoop/pom.xml e280c9eb5 


Diff: https://reviews.apache.org/r/67846/diff/3/


Testing (updated)
---

mvn clean install -Pdist

unpacked the sentry jars that have been shaded to verify contents.


Thanks,

Steve Moist

Re: Review Request 67846: SENTRY-2283 Multiple versions of metrics on the classpath causes Sentry to not startup

[Steve Moist via Review Board]

> On July 9, 2018, 5:53 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathDeltaRetriever.java
> > Line 20 (original), 20 (patched)
> > 
> >
> > Shouldn't this be done automatically by the shading plugin?

No because it's a transitive dependency.


- Steve


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


On July 9, 2018, 7:17 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67846/
> ---
> 
> (Updated July 9, 2018, 7:17 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Excluding conflicting dependencies and shading io.dropwizard.metrics.
> 
> 
> Diffs
> -
> 
>   pom.xml 488426593 
>   sentry-binding/sentry-binding-kafka/pom.xml 239eeba5f 
>   sentry-dist/src/license/THIRD-PARTY.properties b39e1b6ca 
>   sentry-dist/src/main/assembly/bin.xml 72773df1e 
>   sentry-hdfs/sentry-hdfs-dist/pom.xml 93943157d 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathDeltaRetriever.java
>  81c614a34 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java
>  3532ef33d 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermDeltaRetriever.java
>  8d6713acd 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java
>  b87d29040 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java
>  0cd405b54 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsMetricsUtil.java
>  3bf300bef 
>   sentry-provider/sentry-provider-db/pom.xml a8a15bfb1 
>   sentry-service/sentry-service-server/pom.xml a103c1e25 
>   sentry-tests/sentry-tests-solr/pom.xml db33ee9f4 
>   sentry-tests/sentry-tests-sqoop/pom.xml e280c9eb5 
> 
> 
> Diff: https://reviews.apache.org/r/67846/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve Moist
> 
>

Re: Review Request 67846: SENTRY-2283 Multiple versions of metrics on the classpath causes Sentry to not startup

[Steve Moist via Review Board]

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

(Updated July 9, 2018, 7:17 p.m.)


Review request for sentry.


Repository: sentry


Description
---

Excluding conflicting dependencies and shading io.dropwizard.metrics.


Diffs (updated)
-

  pom.xml 488426593 
  sentry-binding/sentry-binding-kafka/pom.xml 239eeba5f 
  sentry-dist/src/license/THIRD-PARTY.properties b39e1b6ca 
  sentry-dist/src/main/assembly/bin.xml 72773df1e 
  sentry-hdfs/sentry-hdfs-dist/pom.xml 93943157d 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathDeltaRetriever.java
 81c614a34 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java
 3532ef33d 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermDeltaRetriever.java
 8d6713acd 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java
 b87d29040 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java
 0cd405b54 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsMetricsUtil.java
 3bf300bef 
  sentry-provider/sentry-provider-db/pom.xml a8a15bfb1 
  sentry-service/sentry-service-server/pom.xml a103c1e25 
  sentry-tests/sentry-tests-solr/pom.xml db33ee9f4 
  sentry-tests/sentry-tests-sqoop/pom.xml e280c9eb5 


Diff: https://reviews.apache.org/r/67846/diff/3/

Changes: https://reviews.apache.org/r/67846/diff/2-3/


Testing
---


Thanks,

Steve Moist

Re: Review Request 67846: SENTRY-2283 Multiple versions of metrics on the classpath causes Sentry to not startup

[Steve Moist via Review Board]

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

(Updated July 6, 2018, 7:06 p.m.)


Review request for sentry.


Repository: sentry


Description
---

Excluding conflicting dependencies and shading io.dropwizard.metrics.


Diffs (updated)
-

  pom.xml 488426593 
  sentry-binding/sentry-binding-hive-follower/pom.xml 5f8a5afb4 
  sentry-binding/sentry-binding-kafka/pom.xml 239eeba5f 
  sentry-dist/src/license/THIRD-PARTY.properties b39e1b6ca 
  sentry-dist/src/main/assembly/bin.xml 72773df1e 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathDeltaRetriever.java
 81c614a34 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java
 3532ef33d 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermDeltaRetriever.java
 8d6713acd 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java
 b87d29040 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java
 0cd405b54 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsMetricsUtil.java
 3bf300bef 
  sentry-provider/sentry-provider-db/pom.xml a8a15bfb1 
  sentry-service/sentry-service-server/pom.xml a103c1e25 
  sentry-tests/sentry-tests-solr/pom.xml db33ee9f4 
  sentry-tests/sentry-tests-sqoop/pom.xml e280c9eb5 


Diff: https://reviews.apache.org/r/67846/diff/2/

Changes: https://reviews.apache.org/r/67846/diff/1-2/


Testing
---


Thanks,

Steve Moist

Review Request 67846: SENTRY-2283 Multiple versions of metrics on the classpath causes Sentry to not startup

[Steve Moist via Review Board]

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

Review request for sentry.


Repository: sentry


Description
---

Excluding conflicting dependencies and shading io.dropwizard.metrics.


Diffs
-

  sentry-binding/sentry-binding-hive-common/pom.xml e154cdee7 
  sentry-binding/sentry-binding-hive-conf/pom.xml 3e7e70a32 
  sentry-binding/sentry-binding-hive-follower/pom.xml 5f8a5afb4 
  sentry-binding/sentry-binding-hive/pom.xml 09d75f7de 
  sentry-binding/sentry-binding-kafka/pom.xml 239eeba5f 
  sentry-dist/src/license/THIRD-PARTY.properties b39e1b6ca 
  sentry-dist/src/main/assembly/bin.xml 72773df1e 
  sentry-provider/sentry-provider-db/pom.xml a8a15bfb1 
  sentry-service/sentry-service-server/pom.xml a103c1e25 
  sentry-tests/sentry-tests-kafka/pom.xml 03bc57453 
  sentry-tests/sentry-tests-solr/pom.xml db33ee9f4 
  sentry-tests/sentry-tests-sqoop/pom.xml e280c9eb5 


Diff: https://reviews.apache.org/r/67846/diff/1/


Testing
---


Thanks,

Steve Moist

Re: Review Request 67093: SENTRY-2208: Refactor out Sentry service into own module from sentry-provider-db

[Steve Moist via Review Board]

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


Ship it!




Ship It!

- Steve Moist


On May 25, 2018, 1:59 p.m., Anthony Young-Garner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67093/
> ---
> 
> (Updated May 25, 2018, 1:59 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-2208
> https://issues.apache.org/jira/browse/SENTRY-2208
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Refactored the SentryService class and its dependencies from 
> sentry-provider/sentry-provider-db into a new 
> sentry-service/sentry-service-server module. In addition, refactored 
> SentryServiceClientFactory into a new sentry-service/sentry-service-client 
> module.
> 
> Work is based on previously completed work on SENTRY-1205.
> 
> NOTE: The diff looks large, but the overwhelming majority (9 pages) of the 
> diff consists of file moves (from sentry-provider-db to sentry-service-server)
> 
> https://issues.apache.org/jira/browse/SENTRY-2208
> 
> 
> Diffs
> -
> 
>   sentry-hdfs/sentry-hdfs-service/pom.xml 
> 50a451d917eb7428cd71ab7cedd9c4363111c166 
>   sentry-provider/sentry-provider-db/pom.xml 
> 48a187ae54afd435c1f3d4aa57b0d696cd634fbe 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/generic/thrift/SentryGenericPolicyProcessor.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/generic/thrift/SentryGenericPolicyProcessorFactory.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/ConfServlet.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/LogLevelServlet.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/PubSubServlet.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryAdminServlet.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryAuthFilter.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryHealthCheckServletContextListener.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryMetricsServletContextListener.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessorFactory.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryWebServer.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SentryPolicyStorePlugin.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeObject.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java
>  876ee146ab31aaa82b83e88585c45bb2deabc0d2 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/GenericPrivilegeConverter.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/TSentryPrivilegeConverter.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/appender/AuditLoggerTestAppender.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/appender/RollingFileWithoutDeleteAppender.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/AuditMetadataLogEntity.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/DBAuditMetadataLogEntity.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/GMAuditMetadataLogEntity.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntity.java
>   
>   
> 

Re: Review Request 67180: SENTRY-2235: Add hive tests to verify column privileges for views

[Steve Moist via Review Board]

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




sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java
Lines 226 (patched)


Why have a if(revoke) if th eonly thing being passed in is true?


- Steve Moist


On May 17, 2018, 12:21 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67180/
> ---
> 
> (Updated May 17, 2018, 12:21 p.m.)
> 
> 
> Review request for sentry, Na Li and Sergio Pena.
> 
> 
> Bugs: SENTRY-2235
> https://issues.apache.org/jira/browse/SENTRY-2235
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> There are no e2e tests that verify column permissions granted on views. New 
> test is added to verify the same.
> 
> 
> Diffs
> -
> 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java
>  35f41c6efc15afe566c760ac0e524946e4725991 
> 
> 
> Diff: https://reviews.apache.org/r/67180/diff/1/
> 
> 
> Testing
> ---
> 
> Added new tests and also made sure that all regresstion passed.
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Re: Review Request 67093: SENTRY-2208: Refactor out Sentry service into own module from sentry-provider-db

[Steve Moist via Review Board]

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



Can't we tease apart that database models and possibly JPA layer stuff back 
into sentry-provider-db then have sentry-service-server depend on that?


sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java
Line 201 (original), 202 (patched)


Convert back to EventType.valueOf.  It's more clear.



sentry-service/sentry-service-client/pom.xml
Lines 62 (patched)


Nit: Too many empty lines, remove.



sentry-service/sentry-service-server/pom.xml
Lines 113 (patched)


Why is this commented out, why not just remove it?


- Steve Moist


On May 15, 2018, 6:47 p.m., Anthony Young-Garner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67093/
> ---
> 
> (Updated May 15, 2018, 6:47 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-2208
> https://issues.apache.org/jira/browse/SENTRY-2208
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Refactored the SentryService class and its dependencies from 
> sentry-provider/sentry-provider-db into a new 
> sentry-service/sentry-service-server module. In addition, refactored 
> SentryServiceClientFactory into a new sentry-service/sentry-service-client 
> module.
> 
> Work is based on previously completed work on SENTRY-1205.
> 
> NOTE: The diff looks large, but the overwhelming majority (9 pages) of the 
> diff consists of file moves (from sentry-provider-db to sentry-service-server)
> 
> https://issues.apache.org/jira/browse/SENTRY-2208
> 
> 
> Diffs
> -
> 
>   sentry-hdfs/sentry-hdfs-service/pom.xml 
> 50a451d917eb7428cd71ab7cedd9c4363111c166 
>   sentry-provider/sentry-provider-db/pom.xml 
> 48a187ae54afd435c1f3d4aa57b0d696cd634fbe 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/generic/thrift/SentryGenericPolicyProcessor.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/generic/thrift/SentryGenericPolicyProcessorFactory.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/ConfServlet.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/LogLevelServlet.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/PubSubServlet.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryAdminServlet.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryAuthFilter.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryHealthCheckServletContextListener.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryMetricsServletContextListener.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessorFactory.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryWebServer.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SentryPolicyStorePlugin.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeObject.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java
>  876ee146ab31aaa82b83e88585c45bb2deabc0d2 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/GenericPrivilegeConverter.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/TSentryPrivilegeConverter.java
>   
>   
> 

Re: Review Request 67093: SENTRY-2208: Refactor out Sentry service into own module from sentry-provider-db

[Steve Moist via Review Board]

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




sentry-service/sentry-service-server/src/main/resources/001-SENTRY-327.derby.sql
Lines 1 (patched)


Why are these sql files tracked as new?  Shouldn't be moved?



sentry-tests/sentry-tests-solr/pom.xml
Lines 65 (patched)


Why is this all included.


- Steve Moist


On May 15, 2018, 6:47 p.m., Anthony Young-Garner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67093/
> ---
> 
> (Updated May 15, 2018, 6:47 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-2208
> https://issues.apache.org/jira/browse/SENTRY-2208
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Refactored the SentryService class and its dependencies from 
> sentry-provider/sentry-provider-db into a new 
> sentry-service/sentry-service-server module. In addition, refactored 
> SentryServiceClientFactory into a new sentry-service/sentry-service-client 
> module.
> 
> Work is based on previously completed work on SENTRY-1205.
> 
> NOTE: The diff looks large, but the overwhelming majority (9 pages) of the 
> diff consists of file moves (from sentry-provider-db to sentry-service-server)
> 
> https://issues.apache.org/jira/browse/SENTRY-2208
> 
> 
> Diffs
> -
> 
>   sentry-hdfs/sentry-hdfs-service/pom.xml 
> 50a451d917eb7428cd71ab7cedd9c4363111c166 
>   sentry-provider/sentry-provider-db/pom.xml 
> 48a187ae54afd435c1f3d4aa57b0d696cd634fbe 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/generic/thrift/SentryGenericPolicyProcessor.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/generic/thrift/SentryGenericPolicyProcessorFactory.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/ConfServlet.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/LogLevelServlet.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/PubSubServlet.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryAdminServlet.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryAuthFilter.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryHealthCheckServletContextListener.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryMetricsServletContextListener.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessorFactory.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/api/service/thrift/SentryWebServer.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SentryPolicyStorePlugin.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeObject.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java
>  876ee146ab31aaa82b83e88585c45bb2deabc0d2 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/GenericPrivilegeConverter.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/TSentryPrivilegeConverter.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/appender/AuditLoggerTestAppender.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/appender/RollingFileWithoutDeleteAppender.java
>   
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/AuditMetadataLogEntity.java
>   
>   
> 

Re: Review Request 66888: SENTRY-2206 Refactor out sentry api from sentry-provider-db to own module

[Steve Moist via Review Board]

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

(Updated May 9, 2018, 8:12 p.m.)


Review request for sentry.


Repository: sentry


Description
---

Mainly refactored classes around to new module 
sentry-service/sentry-service-api.  The ServiceConstants were refactored into 
sentry-core-common as it introduced circular dependencies.  ApiConstants was 
created to split out from ServiceConstants for the API.  A few class constants 
were added into ApiConstants due to circular dependencies.

https://issues.apache.org/jira/browse/SENTRY-2206


Diffs (updated)
-

  pom.xml 262a9d8f 
  
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
 71d12253 
  
sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
 7565a34b 
  
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java
 1dc8f016 
  
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/authorizer/DefaultSentryAccessController.java
 13ee2cfd 
  
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetastorePostEventListenerBaseV2.java
 567e9fa6 
  
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/util/SentryAuthorizerUtil.java
 35bd68ce 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java
 PRE-CREATION 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java
 2abe37e5 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java
 c23547a9 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java
 24d77634 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/util/SentryAuthorizerUtil.java
 1c416393 
  
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/metastore/TestSentrySyncHMSNotificationsPostEventListener.java
 cca326b3 
  
sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java
 e4abdc71 
  
sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java
 5c2a301d 
  
sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java
 b7cbd323 
  
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/api/common/ApiConstants.java
 PRE-CREATION 
  sentry-dist/src/license/THIRD-PARTY.properties 2f9f0b08 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsMetricsUtil.java
 932a5c04 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
 fa83589a 
  sentry-provider/sentry-provider-db/pom.xml 369e262e 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyService.java
 bc8d7e34 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleAddGroupsRequest.java
 2465a889 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleAddGroupsResponse.java
 bb238baa 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleDeleteGroupsRequest.java
 ab8d9d1b 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleDeleteGroupsResponse.java
 68c5cf16 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleGrantPrivilegeRequest.java
 0a10883b 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleGrantPrivilegeResponse.java
 c3808035 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleRevokePrivilegeRequest.java
 c051b090 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleRevokePrivilegeResponse.java
 a2326963 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAuthorizable.java
 0e0cdebd 
  

Re: Review Request 66888: SENTRY-2206 Refactor out sentry api from sentry-provider-db to own module

[Steve Moist via Review Board]

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




sentry-provider/sentry-provider-db/src/main/resources/sentry_generic_policy_service.thrift
Line 29 (original), 29 (patched)


change namespaces



sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift
Line 27 (original), 27 (patched)


change the others namespace.

also do it in common.


- Steve Moist


On May 1, 2018, 7:50 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66888/
> ---
> 
> (Updated May 1, 2018, 7:50 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Mainly refactored classes around to new module 
> sentry-service/sentry-service-api.  The ServiceConstants were refactored into 
> sentry-core-common as it introduced circular dependencies.  ApiConstants was 
> created to split out from ServiceConstants for the API.  A few class 
> constants were added into ApiConstants due to circular dependencies.
> 
> https://issues.apache.org/jira/browse/SENTRY-2206
> 
> 
> Diffs
> -
> 
>   pom.xml 262a9d8f 
>   
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
>  71d12253 
>   
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
>  7565a34b 
>   
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java
>  1dc8f016 
>   
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/authorizer/DefaultSentryAccessController.java
>  13ee2cfd 
>   
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetastorePostEventListenerBaseV2.java
>  567e9fa6 
>   
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/util/SentryAuthorizerUtil.java
>  35bd68ce 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java
>  21a6abf8 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java
>  3ac49fa6 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java
>  c23547a9 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java
>  24d77634 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/util/SentryAuthorizerUtil.java
>  1c416393 
>   
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/metastore/TestSentrySyncHMSNotificationsPostEventListener.java
>  cca326b3 
>   
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java
>  e4abdc71 
>   
> sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java
>  5c2a301d 
>   
> sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java
>  b7cbd323 
>   
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/api/common/ApiConstants.java
>  PRE-CREATION 
>   sentry-dist/src/license/THIRD-PARTY.properties 2f9f0b08 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsMetricsUtil.java
>  932a5c04 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
>  8485ca37 
>   sentry-provider/sentry-provider-db/pom.xml 369e262e 
>   
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyService.java
>  bc8d7e34 
>   
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleAddGroupsRequest.java
>  2465a889 
>   
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleAddGroupsResponse.java
>  bb238baa 
>   
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleDeleteGroupsRequest.java
>  ab8d9d1b 
>   
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleDeleteGroupsResponse.java
>  68c5cf16 
>   
> 

Re: Review Request 66888: SENTRY-2206 Refactor out sentry api from sentry-provider-db to own module

[Steve Moist via Review Board]

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

(Updated May 1, 2018, 7:50 p.m.)


Review request for sentry.


Repository: sentry


Description (updated)
---

Mainly refactored classes around to new module 
sentry-service/sentry-service-api.  The ServiceConstants were refactored into 
sentry-core-common as it introduced circular dependencies.  ApiConstants was 
created to split out from ServiceConstants for the API.  A few class constants 
were added into ApiConstants due to circular dependencies.

https://issues.apache.org/jira/browse/SENTRY-2206


Diffs
-

  pom.xml 262a9d8f 
  
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
 71d12253 
  
sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
 7565a34b 
  
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java
 1dc8f016 
  
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/authorizer/DefaultSentryAccessController.java
 13ee2cfd 
  
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetastorePostEventListenerBaseV2.java
 567e9fa6 
  
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/util/SentryAuthorizerUtil.java
 35bd68ce 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java
 21a6abf8 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java
 3ac49fa6 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java
 c23547a9 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java
 24d77634 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/util/SentryAuthorizerUtil.java
 1c416393 
  
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/metastore/TestSentrySyncHMSNotificationsPostEventListener.java
 cca326b3 
  
sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java
 e4abdc71 
  
sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java
 5c2a301d 
  
sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java
 b7cbd323 
  
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/api/common/ApiConstants.java
 PRE-CREATION 
  sentry-dist/src/license/THIRD-PARTY.properties 2f9f0b08 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsMetricsUtil.java
 932a5c04 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
 8485ca37 
  sentry-provider/sentry-provider-db/pom.xml 369e262e 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyService.java
 bc8d7e34 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleAddGroupsRequest.java
 2465a889 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleAddGroupsResponse.java
 bb238baa 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleDeleteGroupsRequest.java
 ab8d9d1b 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleDeleteGroupsResponse.java
 68c5cf16 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleGrantPrivilegeRequest.java
 0a10883b 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleGrantPrivilegeResponse.java
 c3808035 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleRevokePrivilegeRequest.java
 c051b090 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleRevokePrivilegeResponse.java
 a2326963 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAuthorizable.java
 0e0cdebd 
  

Review Request 66888: SENTRY-2206 Refactor out sentry api from sentry-provider-db to own module

[Steve Moist via Review Board]

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

Review request for sentry.


Repository: sentry


Description
---

Mainly refactored classes around to new module 
sentry-service/sentry-service-api.  The ServiceConstants were refactored into 
sentry-core-common as it introduced circular dependencies.  ApiConstants was 
created to split out from ServiceConstants for the API.  A few class constants 
were added into ApiConstants due to circular dependencies.


Diffs
-

  pom.xml 262a9d8f 
  
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
 71d12253 
  
sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
 7565a34b 
  
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java
 1dc8f016 
  
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/authorizer/DefaultSentryAccessController.java
 13ee2cfd 
  
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetastorePostEventListenerBaseV2.java
 567e9fa6 
  
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/util/SentryAuthorizerUtil.java
 35bd68ce 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java
 21a6abf8 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java
 3ac49fa6 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java
 c23547a9 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java
 24d77634 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/util/SentryAuthorizerUtil.java
 1c416393 
  
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/metastore/TestSentrySyncHMSNotificationsPostEventListener.java
 cca326b3 
  
sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java
 e4abdc71 
  
sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java
 5c2a301d 
  
sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java
 b7cbd323 
  
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/api/common/ApiConstants.java
 PRE-CREATION 
  sentry-dist/src/license/THIRD-PARTY.properties 2f9f0b08 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsMetricsUtil.java
 932a5c04 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
 8485ca37 
  sentry-provider/sentry-provider-db/pom.xml 369e262e 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyService.java
 bc8d7e34 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleAddGroupsRequest.java
 2465a889 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleAddGroupsResponse.java
 bb238baa 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleDeleteGroupsRequest.java
 ab8d9d1b 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleDeleteGroupsResponse.java
 68c5cf16 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleGrantPrivilegeRequest.java
 0a10883b 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleGrantPrivilegeResponse.java
 c3808035 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleRevokePrivilegeRequest.java
 c051b090 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleRevokePrivilegeResponse.java
 a2326963 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAuthorizable.java
 0e0cdebd 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TCreateSentryRoleRequest.java
 e6ed656c 
  

Re: Review Request 66787: SENTRY-2212 smart-apply-patch.sh isn't so smart, won't apply changes when files have been moved or renamed

[Steve Moist via Review Board]

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

(Updated April 27, 2018, 4:47 p.m.)


Review request for sentry.


Changes
---

Copying Hive's version.


Repository: sentry


Description
---

Removed using the patch command and instead am using "git apply" for diffs.


Diffs (updated)
-

  dev-support/smart-apply-patch.sh fce27354 


Diff: https://reviews.apache.org/r/66787/diff/2/

Changes: https://reviews.apache.org/r/66787/diff/1-2/


Testing
---

Used it to apply a patch, then tested with apply a patch with conflicts, doing 
a dry-run.  Also tested with rename/moves.


Thanks,

Steve Moist

Re: Review Request 66748: SENTRY-2207 Refactor out Sentry CLI from sentry-provider-db into own module

[Steve Moist via Review Board]

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

(Updated April 27, 2018, 3:47 p.m.)


Review request for sentry.


Repository: sentry


Description
---

Moved the sentry cli to sentry-tools.  Had to change a dependency in 
sentry-provider-db to re-use the integration base.


Diffs (updated)
-

  bin/sentryShell 17b1429f 
  pom.xml 16a3838a 
  sentry-provider/sentry-provider-db/pom.xml b8cccfa8 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java
 cf552b16 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java
 edf09346 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/GenericPrivilegeConverter.java
 8de543c4 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolCommon.java
 e3d81f80 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolSolr.java
 5735 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolCommon.java
 013e824b 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolIndexer.java
 a5996a7b 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java
 1a4692e0 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellGeneric.java
 4487685a 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellIndexer.java
 5bbe7727 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/GenericShellCommand.java
 a792b5cc 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/TSentryPrivilegeConverter.java
 0bfbc442 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaHelper.java
 cf1c7258 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaTool.java
 d75e24bb 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellCommon.java
 c8b2eef3 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java
 785e27df 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/ShellCommand.java
 eeb3a23f 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/CommandUtil.java
 3f0b5fad 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/HiveShellCommand.java
 3abba526 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestPermissionsMigrationToolSolr.java
 69c067fe 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryConfigToolIndexer.java
 4dddf780 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryConfigToolSolr.java
 9e6ff421 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellIndexer.java
 f66eb859 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellKafka.java
 a9234fa8 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSolr.java
 0f4bb62e 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSqoop.java
 cdba4420 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentrySchemaTool.java
 68abf277 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentryShellHive.java
 de8f0433 
  sentry-provider/sentry-provider-db/src/test/resources/indexer_case.ini  
  
sentry-provider/sentry-provider-db/src/test/resources/indexer_config_import_tool.ini
  
  sentry-provider/sentry-provider-db/src/test/resources/indexer_invalid.ini  
  sentry-provider/sentry-provider-db/src/test/resources/solr_case.ini  
  
sentry-provider/sentry-provider-db/src/test/resources/solr_config_import_tool.ini
  
  sentry-provider/sentry-provider-db/src/test/resources/solr_invalid.ini  
  sentry-tools/pom.xml 4d8fc89e 
  sentry-tools/src/main/java/org/apache/sentry/SentryMain.java e92155ce 
  sentry-tools/src/main/java/org/apache/sentry/shell/GroupShell.java b7652a58 
  

Re: Review Request 66748: SENTRY-2207 Refactor out Sentry CLI from sentry-provider-db into own module

[Steve Moist via Review Board]

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

(Updated April 26, 2018, 7:38 p.m.)


Review request for sentry.


Repository: sentry


Description
---

Moved the sentry cli to sentry-tools.  Had to change a dependency in 
sentry-provider-db to re-use the integration base.


Diffs (updated)
-

  bin/sentryShell 17b1429f 
  pom.xml 16a3838a 
  
sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java
 e4abdc71 
  
sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java
 5c2a301d 
  
sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java
 b7cbd323 
  sentry-dist/src/license/THIRD-PARTY.properties 2f9f0b08 
  sentry-provider/sentry-provider-db/pom.xml b8cccfa8 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/GenericPrivilegeConverter.java
 8de543c4 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolCommon.java
 e3d81f80 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolSolr.java
 5735 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolCommon.java
 013e824b 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolIndexer.java
 a5996a7b 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java
 1a4692e0 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellGeneric.java
 4487685a 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellIndexer.java
 5bbe7727 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/GenericShellCommand.java
 a792b5cc 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/TSentryPrivilegeConverter.java
 0bfbc442 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaHelper.java
 cf1c7258 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaTool.java
 d75e24bb 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellCommon.java
 c8b2eef3 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java
 785e27df 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/ShellCommand.java
 eeb3a23f 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/CommandUtil.java
 3f0b5fad 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/HiveShellCommand.java
 3abba526 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestPermissionsMigrationToolSolr.java
 69c067fe 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryConfigToolIndexer.java
 4dddf780 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryConfigToolSolr.java
 9e6ff421 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellIndexer.java
 f66eb859 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellKafka.java
 a9234fa8 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSolr.java
 0f4bb62e 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSqoop.java
 cdba4420 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentrySchemaTool.java
 68abf277 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentryShellHive.java
 de8f0433 
  sentry-provider/sentry-provider-db/src/test/resources/indexer_case.ini  
  
sentry-provider/sentry-provider-db/src/test/resources/indexer_config_import_tool.ini
  
  sentry-provider/sentry-provider-db/src/test/resources/indexer_invalid.ini  
  sentry-provider/sentry-provider-db/src/test/resources/solr_case.ini  
  
sentry-provider/sentry-provider-db/src/test/resources/solr_config_import_tool.ini
  
  sentry-provider/sentry-provider-db/src/test/resources/solr_invalid.ini  
  sentry-tools/pom.xml 4d8fc89e 
  

Re: Review Request 66748: SENTRY-2207 Refactor out Sentry CLI from sentry-provider-db into own module

[Steve Moist via Review Board]

> On April 25, 2018, 4:51 p.m., Colm O hEigeartaigh wrote:
> > Why is the package name changed for GenericPrivilegeConverter.java + 
> > TSentryPrivilegeConverter.java? They've gone from 
> > "sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools"
> >  to "sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools". 
> > Aren't they only applicable to the generic case and if so wouldn't it make 
> > more sense to leave them in the generic package?
> 
> Steve Moist wrote:
> I was moving things around and probably forgot about that one.  they have 
> been moved as part of SENTRY-2206 into the api module as they're primarily 
> used there.
> 
> Colm O hEigeartaigh wrote:
> Could we leave them where they were for this patch and then move them as 
> part of SENTRY-2206? It just makes the patch simpler to review as it only 
> deals with one thing then.

Sure thing.


- Steve


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


On April 24, 2018, 7:29 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66748/
> ---
> 
> (Updated April 24, 2018, 7:29 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Moved the sentry cli to sentry-tools.  Had to change a dependency in 
> sentry-provider-db to re-use the integration base.
> 
> 
> Diffs
> -
> 
>   bin/sentryShell 17b1429f 
>   pom.xml 16a3838a 
>   
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java
>  e4abdc71 
>   
> sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java
>  5c2a301d 
>   
> sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java
>  b7cbd323 
>   sentry-dist/src/license/THIRD-PARTY.properties 2f9f0b08 
>   sentry-provider/sentry-provider-db/pom.xml b8cccfa8 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java
>  cf552b16 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java
>  edf09346 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/GenericPrivilegeConverter.java
>  8de543c4 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolCommon.java
>  e3d81f80 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolSolr.java
>  5735 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolCommon.java
>  013e824b 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolIndexer.java
>  a5996a7b 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java
>  1a4692e0 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellGeneric.java
>  4487685a 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellIndexer.java
>  5bbe7727 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/GenericShellCommand.java
>  a792b5cc 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/TSentryPrivilegeConverter.java
>  0bfbc442 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaHelper.java
>  cf1c7258 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaTool.java
>  d75e24bb 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellCommon.java
>  c8b2eef3 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java
>  785e27df 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/ShellCommand.java
>  eeb3a23f 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/CommandUtil.java
>  3f0b5fad 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/HiveShellCommand.java
>  3abba526 
>   
> 

Re: Review Request 66787: SENTRY-2212 smart-apply-patch.sh isn't so smart, won't apply changes when files have been moved or renamed

[Steve Moist via Review Board]

> On April 25, 2018, 7:58 p.m., Sergio Pena wrote:
> > I found out that Hive made a change on the smart-apply-patch.sh to use git 
> > apply instead of patch, and the patch still have some validation to check 
> > if the patch was generated using the --no-prefix and also the levels.
> > See 
> > https://github.com/apache/hive/blob/master/testutils/ptest2/src/main/resources/smart-apply-patch.sh
> > 
> > Should we do something similar? Just replace patch for git apply? or do you 
> > think it is not neccessary?

That looks pretty good, I'll take a look at it and why it's doing that instead 
of a straight git apply.


- Steve


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


On April 24, 2018, 9:18 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66787/
> ---
> 
> (Updated April 24, 2018, 9:18 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Removed using the patch command and instead am using "git apply" for diffs.
> 
> 
> Diffs
> -
> 
>   dev-support/smart-apply-patch.sh fce27354 
> 
> 
> Diff: https://reviews.apache.org/r/66787/diff/1/
> 
> 
> Testing
> ---
> 
> Used it to apply a patch, then tested with apply a patch with conflicts, 
> doing a dry-run.  Also tested with rename/moves.
> 
> 
> Thanks,
> 
> Steve Moist
> 
>

Re: Review Request 66787: SENTRY-2212 smart-apply-patch.sh isn't so smart, won't apply changes when files have been moved or renamed

[Steve Moist via Review Board]

> On April 25, 2018, 4:07 p.m., Anthony Young-Garner wrote:
> > dev-support/smart-apply-patch.sh
> > Line 44 (original)
> > 
> >
> > Do we not still want to detect this situation (only new files) and try 
> > applying the patch at levels 0,1,2? (Lines 44 - 85)
> 
> Steve Moist wrote:
> No, we should just use git apply to be consistant.
> 
> Alexander Kolbasov wrote:
> A lot of people generate patches that are not applyable with git apply.

Do you have an example of it?


- Steve


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


On April 24, 2018, 9:18 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66787/
> ---
> 
> (Updated April 24, 2018, 9:18 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Removed using the patch command and instead am using "git apply" for diffs.
> 
> 
> Diffs
> -
> 
>   dev-support/smart-apply-patch.sh fce27354 
> 
> 
> Diff: https://reviews.apache.org/r/66787/diff/1/
> 
> 
> Testing
> ---
> 
> Used it to apply a patch, then tested with apply a patch with conflicts, 
> doing a dry-run.  Also tested with rename/moves.
> 
> 
> Thanks,
> 
> Steve Moist
> 
>

Re: Review Request 66748: SENTRY-2207 Refactor out Sentry CLI from sentry-provider-db into own module

[Steve Moist via Review Board]

> On April 25, 2018, 4:51 p.m., Colm O hEigeartaigh wrote:
> > Why is the package name changed for GenericPrivilegeConverter.java + 
> > TSentryPrivilegeConverter.java? They've gone from 
> > "sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools"
> >  to "sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools". 
> > Aren't they only applicable to the generic case and if so wouldn't it make 
> > more sense to leave them in the generic package?

I was moving things around and probably forgot about that one.  they have been 
moved as part of SENTRY-2206 into the api module as they're primarily used 
there.


- Steve


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


On April 24, 2018, 7:29 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66748/
> ---
> 
> (Updated April 24, 2018, 7:29 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Moved the sentry cli to sentry-tools.  Had to change a dependency in 
> sentry-provider-db to re-use the integration base.
> 
> 
> Diffs
> -
> 
>   bin/sentryShell 17b1429f 
>   pom.xml 16a3838a 
>   
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java
>  e4abdc71 
>   
> sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java
>  5c2a301d 
>   
> sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java
>  b7cbd323 
>   sentry-dist/src/license/THIRD-PARTY.properties 2f9f0b08 
>   sentry-provider/sentry-provider-db/pom.xml b8cccfa8 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java
>  cf552b16 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java
>  edf09346 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/GenericPrivilegeConverter.java
>  8de543c4 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolCommon.java
>  e3d81f80 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolSolr.java
>  5735 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolCommon.java
>  013e824b 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolIndexer.java
>  a5996a7b 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java
>  1a4692e0 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellGeneric.java
>  4487685a 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellIndexer.java
>  5bbe7727 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/GenericShellCommand.java
>  a792b5cc 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/TSentryPrivilegeConverter.java
>  0bfbc442 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaHelper.java
>  cf1c7258 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaTool.java
>  d75e24bb 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellCommon.java
>  c8b2eef3 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java
>  785e27df 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/ShellCommand.java
>  eeb3a23f 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/CommandUtil.java
>  3f0b5fad 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/HiveShellCommand.java
>  3abba526 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestPermissionsMigrationToolSolr.java
>  69c067fe 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryConfigToolIndexer.java
>  4dddf780 
>   
> 

Re: Review Request 66787: SENTRY-2212 smart-apply-patch.sh isn't so smart, won't apply changes when files have been moved or renamed

[Steve Moist via Review Board]

> On April 25, 2018, 4:07 p.m., Anthony Young-Garner wrote:
> > dev-support/smart-apply-patch.sh
> > Line 33 (original), 32 (patched)
> > 
> >
> > Will we not continue to support stdin patches? I think it can still be 
> > achieved with git.

It can, this is a bit simpler to just read from a file. I can add it back if 
needed.  Does anyone know if it's being used this way?


> On April 25, 2018, 4:07 p.m., Anthony Young-Garner wrote:
> > dev-support/smart-apply-patch.sh
> > Line 44 (original)
> > 
> >
> > Do we not still want to detect this situation (only new files) and try 
> > applying the patch at levels 0,1,2? (Lines 44 - 85)

No, we should just use git apply to be consistant.


- Steve


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


On April 24, 2018, 9:18 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66787/
> ---
> 
> (Updated April 24, 2018, 9:18 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Removed using the patch command and instead am using "git apply" for diffs.
> 
> 
> Diffs
> -
> 
>   dev-support/smart-apply-patch.sh fce27354 
> 
> 
> Diff: https://reviews.apache.org/r/66787/diff/1/
> 
> 
> Testing
> ---
> 
> Used it to apply a patch, then tested with apply a patch with conflicts, 
> doing a dry-run.  Also tested with rename/moves.
> 
> 
> Thanks,
> 
> Steve Moist
> 
>

Re: Review Request 66787: SENTRY-2212 smart-apply-patch.sh isn't so smart, won't apply changes when files have been moved or renamed

[Steve Moist via Review Board]

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




dev-support/smart-apply-patch.sh
Line 33 (original), 32 (patched)


It can, this is a bit simpler to just read from a file. I can add it back 
if needed.  Does anyone know if it's being used this way?


- Steve Moist


On April 24, 2018, 9:18 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66787/
> ---
> 
> (Updated April 24, 2018, 9:18 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Removed using the patch command and instead am using "git apply" for diffs.
> 
> 
> Diffs
> -
> 
>   dev-support/smart-apply-patch.sh fce27354 
> 
> 
> Diff: https://reviews.apache.org/r/66787/diff/1/
> 
> 
> Testing
> ---
> 
> Used it to apply a patch, then tested with apply a patch with conflicts, 
> doing a dry-run.  Also tested with rename/moves.
> 
> 
> Thanks,
> 
> Steve Moist
> 
>

Review Request 66787: SENTRY-2212 smart-apply-patch.sh isn't so smart, won't apply changes when files have been moved or renamed

[Steve Moist via Review Board]

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

Review request for sentry.


Repository: sentry


Description
---

Removed using the patch command and instead am using "git apply" for diffs.


Diffs
-

  dev-support/smart-apply-patch.sh fce27354 


Diff: https://reviews.apache.org/r/66787/diff/1/


Testing
---

Used it to apply a patch, then tested with apply a patch with conflicts, doing 
a dry-run.  Also tested with rename/moves.


Thanks,

Steve Moist

Re: Review Request 66748: SENTRY-2207 Refactor out Sentry CLI from sentry-provider-db into own module

[Steve Moist via Review Board]

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

(Updated April 24, 2018, 7:29 p.m.)


Review request for sentry.


Repository: sentry


Description
---

Moved the sentry cli to sentry-tools.  Had to change a dependency in 
sentry-provider-db to re-use the integration base.


Diffs (updated)
-

  bin/sentryShell 17b1429f 
  pom.xml 16a3838a 
  
sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java
 e4abdc71 
  
sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java
 5c2a301d 
  
sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java
 b7cbd323 
  sentry-dist/src/license/THIRD-PARTY.properties 2f9f0b08 
  sentry-provider/sentry-provider-db/pom.xml b8cccfa8 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java
 cf552b16 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java
 edf09346 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/GenericPrivilegeConverter.java
 8de543c4 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolCommon.java
 e3d81f80 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolSolr.java
 5735 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolCommon.java
 013e824b 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolIndexer.java
 a5996a7b 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java
 1a4692e0 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellGeneric.java
 4487685a 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellIndexer.java
 5bbe7727 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/GenericShellCommand.java
 a792b5cc 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/TSentryPrivilegeConverter.java
 0bfbc442 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaHelper.java
 cf1c7258 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaTool.java
 d75e24bb 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellCommon.java
 c8b2eef3 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java
 785e27df 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/ShellCommand.java
 eeb3a23f 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/CommandUtil.java
 3f0b5fad 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/HiveShellCommand.java
 3abba526 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestPermissionsMigrationToolSolr.java
 69c067fe 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryConfigToolIndexer.java
 4dddf780 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryConfigToolSolr.java
 9e6ff421 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellIndexer.java
 f66eb859 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellKafka.java
 a9234fa8 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSolr.java
 0f4bb62e 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSqoop.java
 cdba4420 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentrySchemaTool.java
 68abf277 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentryShellHive.java
 de8f0433 
  sentry-provider/sentry-provider-db/src/test/resources/indexer_case.ini  
  
sentry-provider/sentry-provider-db/src/test/resources/indexer_config_import_tool.ini
  
  sentry-provider/sentry-provider-db/src/test/resources/indexer_invalid.ini  
  

Re: Review Request 66748: SENTRY-2207 Refactor out Sentry CLI from sentry-provider-db into own module

[Steve Moist via Review Board]

> On April 23, 2018, 4:34 p.m., Colm O hEigeartaigh wrote:
> > Shouldn't the location of the files be changed as well? If I apply the diff 
> > from this JIRA I see, for example:
> > 
> > ./sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/ShellCommand.java
> > 
> > has the package name:
> > 
> > package org.apache.sentry.cli.tools;
> > 
> > Does the patch need to be updated to actually perform the move into 
> > sentry-tools?
> 
> Steve Moist wrote:
> Odd, that should have been included.  Let me check on that.
> 
> Steve Moist wrote:
> It's a problem with generated a diff and applying the patch.
> 
> Colm O hEigeartaigh wrote:
> The patch still doesn't work for meI'm not sure if the RR is giving 
> me the old diff. Could you update the patch on the JIRA instead?

Colm, what are you using to patch it with?  smart-apply-patch doesn't handle 
renames/moves well as it's using the patch command.  See SENTRY-2212.  I'll try 
and get a patch out again today.


- Steve


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


On April 23, 2018, 8:45 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66748/
> ---
> 
> (Updated April 23, 2018, 8:45 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Moved the sentry cli to sentry-tools.  Had to change a dependency in 
> sentry-provider-db to re-use the integration base.
> 
> 
> Diffs
> -
> 
>   bin/sentryShell 17b1429f 
>   pom.xml 16a3838a 
>   
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java
>  e4abdc71 
>   
> sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java
>  5c2a301d 
>   
> sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java
>  b7cbd323 
>   sentry-provider/sentry-provider-db/pom.xml b8cccfa8 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java
>  cf552b16 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java
>  edf09346 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/GenericPrivilegeConverter.java
>  8de543c4 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolCommon.java
>  e3d81f80 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolSolr.java
>  5735 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolCommon.java
>  013e824b 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolIndexer.java
>  a5996a7b 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java
>  1a4692e0 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellGeneric.java
>  4487685a 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellIndexer.java
>  5bbe7727 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/GenericShellCommand.java
>  a792b5cc 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/TSentryPrivilegeConverter.java
>  0bfbc442 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaHelper.java
>  cf1c7258 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaTool.java
>  d75e24bb 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellCommon.java
>  c8b2eef3 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java
>  785e27df 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/ShellCommand.java
>  eeb3a23f 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/CommandUtil.java
>  3f0b5fad 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/HiveShellCommand.java
>  3abba526 
>   
> 

Re: Review Request 66748: SENTRY-2207 Refactor out Sentry CLI from sentry-provider-db into own module

[Steve Moist via Review Board]

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

(Updated April 23, 2018, 8:45 p.m.)


Review request for sentry.


Summary (updated)
-

SENTRY-2207 Refactor out Sentry CLI from sentry-provider-db into own module


Repository: sentry


Description
---

Moved the sentry cli to sentry-tools.  Had to change a dependency in 
sentry-provider-db to re-use the integration base.


Diffs
-

  bin/sentryShell 17b1429f 
  pom.xml 16a3838a 
  
sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java
 e4abdc71 
  
sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java
 5c2a301d 
  
sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java
 b7cbd323 
  sentry-provider/sentry-provider-db/pom.xml b8cccfa8 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java
 cf552b16 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java
 edf09346 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/GenericPrivilegeConverter.java
 8de543c4 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolCommon.java
 e3d81f80 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolSolr.java
 5735 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolCommon.java
 013e824b 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolIndexer.java
 a5996a7b 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java
 1a4692e0 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellGeneric.java
 4487685a 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellIndexer.java
 5bbe7727 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/GenericShellCommand.java
 a792b5cc 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/TSentryPrivilegeConverter.java
 0bfbc442 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaHelper.java
 cf1c7258 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaTool.java
 d75e24bb 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellCommon.java
 c8b2eef3 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java
 785e27df 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/ShellCommand.java
 eeb3a23f 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/CommandUtil.java
 3f0b5fad 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/HiveShellCommand.java
 3abba526 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestPermissionsMigrationToolSolr.java
 69c067fe 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryConfigToolIndexer.java
 4dddf780 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryConfigToolSolr.java
 9e6ff421 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellIndexer.java
 f66eb859 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellKafka.java
 a9234fa8 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSolr.java
 0f4bb62e 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSqoop.java
 cdba4420 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentrySchemaTool.java
 68abf277 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentryShellHive.java
 de8f0433 
  sentry-tools/pom.xml 4d8fc89e 
  sentry-tools/src/main/java/org/apache/sentry/SentryMain.java e92155ce 
  sentry-tools/src/main/java/org/apache/sentry/shell/GroupShell.java b7652a58 
  sentry-tools/src/main/java/org/apache/sentry/shell/PrivsShell.java 

Re: Review Request 66748: Refactor out Sentry CLI from sentry-provider-db into own module

[Steve Moist via Review Board]

> On April 23, 2018, 4:34 p.m., Colm O hEigeartaigh wrote:
> > Shouldn't the location of the files be changed as well? If I apply the diff 
> > from this JIRA I see, for example:
> > 
> > ./sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/ShellCommand.java
> > 
> > has the package name:
> > 
> > package org.apache.sentry.cli.tools;
> > 
> > Does the patch need to be updated to actually perform the move into 
> > sentry-tools?
> 
> Steve Moist wrote:
> Odd, that should have been included.  Let me check on that.

It's a problem with generated a diff and applying the patch.


- Steve


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


On April 20, 2018, 9:43 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66748/
> ---
> 
> (Updated April 20, 2018, 9:43 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Moved the sentry cli to sentry-tools.  Had to change a dependency in 
> sentry-provider-db to re-use the integration base.
> 
> 
> Diffs
> -
> 
>   bin/sentryShell 17b1429f 
>   pom.xml 16a3838a 
>   
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java
>  e4abdc71 
>   
> sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java
>  5c2a301d 
>   
> sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java
>  b7cbd323 
>   sentry-provider/sentry-provider-db/pom.xml b8cccfa8 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java
>  cf552b16 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java
>  edf09346 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/GenericPrivilegeConverter.java
>  8de543c4 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolCommon.java
>  e3d81f80 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolSolr.java
>  5735 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolCommon.java
>  013e824b 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolIndexer.java
>  a5996a7b 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java
>  1a4692e0 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellGeneric.java
>  4487685a 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellIndexer.java
>  5bbe7727 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/GenericShellCommand.java
>  a792b5cc 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/TSentryPrivilegeConverter.java
>  0bfbc442 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaHelper.java
>  cf1c7258 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaTool.java
>  d75e24bb 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellCommon.java
>  c8b2eef3 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java
>  785e27df 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/ShellCommand.java
>  eeb3a23f 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/CommandUtil.java
>  3f0b5fad 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/HiveShellCommand.java
>  3abba526 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestPermissionsMigrationToolSolr.java
>  69c067fe 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryConfigToolIndexer.java
>  4dddf780 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryConfigToolSolr.java
>  9e6ff421 
>   
> 

Re: Review Request 66748: Refactor out Sentry CLI from sentry-provider-db into own module

[Steve Moist via Review Board]

> On April 23, 2018, 4:34 p.m., Colm O hEigeartaigh wrote:
> > Shouldn't the location of the files be changed as well? If I apply the diff 
> > from this JIRA I see, for example:
> > 
> > ./sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/ShellCommand.java
> > 
> > has the package name:
> > 
> > package org.apache.sentry.cli.tools;
> > 
> > Does the patch need to be updated to actually perform the move into 
> > sentry-tools?

Odd, that should have been included.  Let me check on that.


- Steve


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


On April 20, 2018, 9:43 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66748/
> ---
> 
> (Updated April 20, 2018, 9:43 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Moved the sentry cli to sentry-tools.  Had to change a dependency in 
> sentry-provider-db to re-use the integration base.
> 
> 
> Diffs
> -
> 
>   bin/sentryShell 17b1429f 
>   pom.xml 16a3838a 
>   
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java
>  e4abdc71 
>   
> sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java
>  5c2a301d 
>   
> sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java
>  b7cbd323 
>   sentry-provider/sentry-provider-db/pom.xml b8cccfa8 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java
>  cf552b16 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java
>  edf09346 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/GenericPrivilegeConverter.java
>  8de543c4 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolCommon.java
>  e3d81f80 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolSolr.java
>  5735 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolCommon.java
>  013e824b 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolIndexer.java
>  a5996a7b 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java
>  1a4692e0 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellGeneric.java
>  4487685a 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellIndexer.java
>  5bbe7727 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/GenericShellCommand.java
>  a792b5cc 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/TSentryPrivilegeConverter.java
>  0bfbc442 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaHelper.java
>  cf1c7258 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaTool.java
>  d75e24bb 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellCommon.java
>  c8b2eef3 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java
>  785e27df 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/ShellCommand.java
>  eeb3a23f 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/CommandUtil.java
>  3f0b5fad 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/HiveShellCommand.java
>  3abba526 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestPermissionsMigrationToolSolr.java
>  69c067fe 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryConfigToolIndexer.java
>  4dddf780 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryConfigToolSolr.java
>  9e6ff421 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellIndexer.java
> 

Review Request 66748: Refactor out Sentry CLI from sentry-provider-db into own module

[Steve Moist via Review Board]

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

Review request for sentry.


Repository: sentry


Description
---

Moved the sentry cli to sentry-tools.  Had to change a dependency in 
sentry-provider-db to re-use the integration base.


Diffs
-

  bin/sentryShell 17b1429f 
  pom.xml 16a3838a 
  
sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java
 e4abdc71 
  
sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java
 5c2a301d 
  
sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java
 b7cbd323 
  sentry-provider/sentry-provider-db/pom.xml b8cccfa8 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java
 cf552b16 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java
 edf09346 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/GenericPrivilegeConverter.java
 8de543c4 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolCommon.java
 e3d81f80 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolSolr.java
 5735 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolCommon.java
 013e824b 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolIndexer.java
 a5996a7b 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java
 1a4692e0 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellGeneric.java
 4487685a 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellIndexer.java
 5bbe7727 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/GenericShellCommand.java
 a792b5cc 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/command/TSentryPrivilegeConverter.java
 0bfbc442 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaHelper.java
 cf1c7258 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentrySchemaTool.java
 d75e24bb 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellCommon.java
 c8b2eef3 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java
 785e27df 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/ShellCommand.java
 eeb3a23f 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/CommandUtil.java
 3f0b5fad 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/HiveShellCommand.java
 3abba526 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestPermissionsMigrationToolSolr.java
 69c067fe 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryConfigToolIndexer.java
 4dddf780 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryConfigToolSolr.java
 9e6ff421 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellIndexer.java
 f66eb859 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellKafka.java
 a9234fa8 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSolr.java
 0f4bb62e 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestSentryShellSqoop.java
 cdba4420 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentrySchemaTool.java
 68abf277 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/tools/TestSentryShellHive.java
 de8f0433 
  sentry-tools/pom.xml 4d8fc89e 
  sentry-tools/src/main/java/org/apache/sentry/SentryMain.java e92155ce 
  sentry-tools/src/main/java/org/apache/sentry/shell/GroupShell.java b7652a58 
  sentry-tools/src/main/java/org/apache/sentry/shell/PrivsShell.java 8b8898fc 
  sentry-tools/src/main/java/org/apache/sentry/shell/RolesShell.java c014a304 
  

Re: Review Request 66590: SENTRY-2201 Initial Attriubte based access control

[Steve Moist via Review Board]

> On April 19, 2018, 10:04 p.m., Na Li wrote:
> > sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java
> > Lines 60 (patched)
> > 
> >
> > should we make this function thread-safe? It invoves two tables and 
> > they have to be consistent.

Not at this time, there should only be 1 thread that would be calling this at 
this time and for the next few revisions.


> On April 19, 2018, 10:04 p.m., Na Li wrote:
> > sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java
> > Lines 101 (patched)
> > 
> >
> > do we need to make this function thread-safe? Otherwise, those two 
> > tables may be in inconsistent state.

Not at this time, there should only be 1 thread that would be calling this at 
this time and for the next few revisions.


> On April 19, 2018, 10:04 p.m., Na Li wrote:
> > sentry-abac/src/main/java/org/apache/sentry/abac/SentryAttributeAuthorizer.java
> > Lines 116 (patched)
> > 
> >
> > should each policy has an index? So you can print the index and it is 
> > easy to debug and correleate events.

It can, once we add profiles it would go better here.


> On April 19, 2018, 10:04 p.m., Na Li wrote:
> > sentry-abac/src/main/java/org/apache/sentry/abac/SentryAttributeAuthorizer.java
> > Lines 119 (patched)
> > 
> >
> > This logic has some problem. will the policies have priorities? then 
> > they should be sorted before you check them and need to handle conflict.
> > 
> > If you don't do that, what's the assumption the policies behave?

Yes, the policies WILL have priorities, for now if there is a label on the 
field it is redacted.  This will be expanded upon in later versions.


> On April 19, 2018, 10:04 p.m., Na Li wrote:
> > sentry-abac/src/main/java/org/apache/sentry/abac/StaticAttributeProvider.java
> > Lines 48 (patched)
> > 
> >
> > do you expect someone calls this function?
> > 
> > It seems you should have a internal thread to periodically pull other 
> > places to get delta change.

Yes, right now static ingestion is provided, this a placeholder for deltas in 
the next ingestion method that will poll for changes.


- Steve


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


On April 19, 2018, 9:09 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66590/
> ---
> 
> (Updated April 19, 2018, 9:09 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> This is the inital draft of attribute based access control.
> 
> 
> Diffs
> -
> 
>   pom.xml 16a3838a 
>   sentry-abac/example-definition.json PRE-CREATION 
>   sentry-abac/example-delta.json PRE-CREATION 
>   sentry-abac/notes.txt PRE-CREATION 
>   sentry-abac/pom.xml PRE-CREATION 
>   sentry-abac/src/main/java/org/apache/sentry/abac/Attribute.java 
> PRE-CREATION 
>   sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java 
> PRE-CREATION 
>   sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapAdapter.java 
> PRE-CREATION 
>   
> sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapKeyException.java
>  PRE-CREATION 
>   
> sentry-abac/src/main/java/org/apache/sentry/abac/GenericAttributeProvider.java
>  PRE-CREATION 
>   
> sentry-abac/src/main/java/org/apache/sentry/abac/SentryAttributeAuthorizer.java
>  PRE-CREATION 
>   sentry-abac/src/main/java/org/apache/sentry/abac/SentryObject.java 
> PRE-CREATION 
>   
> sentry-abac/src/main/java/org/apache/sentry/abac/StaticAttributeProvider.java 
> PRE-CREATION 
>   sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttribute.java 
> PRE-CREATION 
>   
> sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttributeMap.java 
> PRE-CREATION 
>   
> sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestSentryAttributeAuthorizer.java
>  PRE-CREATION 
>   
> sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestStaticProvider.java
>  PRE-CREATION 
>   sentry-abac/src/test/resources/abac.props PRE-CREATION 
>   sentry-binding/sentry-binding-hive/pom.xml ccfa9cfe 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryValidator.java
>  1ab5be35 
>   
> 

Re: Review Request 66590: SENTRY-2201 Initial Attriubte based access control

[Steve Moist via Review Board]

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

(Updated April 20, 2018, 4:14 p.m.)


Review request for sentry.


Repository: sentry


Description
---

This is the inital draft of attribute based access control.


Diffs (updated)
-

  pom.xml 16a3838a 
  sentry-abac/example-definition.json PRE-CREATION 
  sentry-abac/example-delta.json PRE-CREATION 
  sentry-abac/notes.txt PRE-CREATION 
  sentry-abac/pom.xml PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/Attribute.java PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java 
PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapAdapter.java 
PRE-CREATION 
  
sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapKeyException.java 
PRE-CREATION 
  
sentry-abac/src/main/java/org/apache/sentry/abac/GenericAttributeProvider.java 
PRE-CREATION 
  
sentry-abac/src/main/java/org/apache/sentry/abac/SentryAttributeAuthorizer.java 
PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/SentryObject.java 
PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/StaticAttributeProvider.java 
PRE-CREATION 
  sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttribute.java 
PRE-CREATION 
  sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttributeMap.java 
PRE-CREATION 
  
sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestSentryAttributeAuthorizer.java
 PRE-CREATION 
  
sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestStaticProvider.java 
PRE-CREATION 
  sentry-abac/src/test/resources/abac.props PRE-CREATION 
  sentry-binding/sentry-binding-hive/pom.xml ccfa9cfe 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryValidator.java
 1ab5be35 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerImpl.java
 86ff0cc2 


Diff: https://reviews.apache.org/r/66590/diff/6/

Changes: https://reviews.apache.org/r/66590/diff/5-6/


Testing
---

full build,added unit tests, tested code on a cluster.


Thanks,

Steve Moist

Re: Review Request 66590: SENTRY-2201 Initial Attriubte based access control

[Steve Moist via Review Board]

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

(Updated April 19, 2018, 9:09 p.m.)


Review request for sentry.


Repository: sentry


Description
---

This is the inital draft of attribute based access control.


Diffs (updated)
-

  pom.xml 16a3838a 
  sentry-abac/example-definition.json PRE-CREATION 
  sentry-abac/example-delta.json PRE-CREATION 
  sentry-abac/notes.txt PRE-CREATION 
  sentry-abac/pom.xml PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/Attribute.java PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java 
PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapAdapter.java 
PRE-CREATION 
  
sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapKeyException.java 
PRE-CREATION 
  
sentry-abac/src/main/java/org/apache/sentry/abac/GenericAttributeProvider.java 
PRE-CREATION 
  
sentry-abac/src/main/java/org/apache/sentry/abac/SentryAttributeAuthorizer.java 
PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/SentryObject.java 
PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/StaticAttributeProvider.java 
PRE-CREATION 
  sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttribute.java 
PRE-CREATION 
  sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttributeMap.java 
PRE-CREATION 
  
sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestSentryAttributeAuthorizer.java
 PRE-CREATION 
  
sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestStaticProvider.java 
PRE-CREATION 
  sentry-abac/src/test/resources/abac.props PRE-CREATION 
  sentry-binding/sentry-binding-hive/pom.xml ccfa9cfe 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryValidator.java
 1ab5be35 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerImpl.java
 86ff0cc2 


Diff: https://reviews.apache.org/r/66590/diff/5/

Changes: https://reviews.apache.org/r/66590/diff/4-5/


Testing
---

full build,added unit tests, tested code on a cluster.


Thanks,

Steve Moist

Re: Review Request 66590: SENTRY-2201 Initial Attriubte based access control

[Steve Moist via Review Board]

> On April 16, 2018, 2:38 p.m., Xinran Tinney wrote:
> > sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java
> > Lines 117 (patched)
> > 
> >
> > targetObjects.size()=0 might because of line 111, so logically, there 
> > were targets for the attribute actually.

There isn't, if there was 1 remaining Sentry Object for a attribute, we remove 
the sentry object, thus there are no more objects for the attribute, so we 
should remove it.  I've clarified the log message to say remaining in it.


- Steve


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


On April 16, 2018, 7:51 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66590/
> ---
> 
> (Updated April 16, 2018, 7:51 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> This is the inital draft of attribute based access control.
> 
> 
> Diffs
> -
> 
>   pom.xml 16a3838a 
>   sentry-abac/example-definition.json PRE-CREATION 
>   sentry-abac/pom.xml PRE-CREATION 
>   sentry-abac/src/main/java/org/apache/sentry/abac/Attribute.java 
> PRE-CREATION 
>   sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java 
> PRE-CREATION 
>   sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapAdapter.java 
> PRE-CREATION 
>   
> sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapKeyException.java
>  PRE-CREATION 
>   
> sentry-abac/src/main/java/org/apache/sentry/abac/GenericAttributeProvider.java
>  PRE-CREATION 
>   
> sentry-abac/src/main/java/org/apache/sentry/abac/SentryAttributeAuthorizer.java
>  PRE-CREATION 
>   sentry-abac/src/main/java/org/apache/sentry/abac/SentryObject.java 
> PRE-CREATION 
>   
> sentry-abac/src/main/java/org/apache/sentry/abac/StaticAttributeProvider.java 
> PRE-CREATION 
>   sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttribute.java 
> PRE-CREATION 
>   
> sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttributeMap.java 
> PRE-CREATION 
>   
> sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestSentryAttributeAuthorizer.java
>  PRE-CREATION 
>   
> sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestStaticProvider.java
>  PRE-CREATION 
>   sentry-abac/src/test/resources/abac.props PRE-CREATION 
>   sentry-binding/sentry-binding-hive/pom.xml ccfa9cfe 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryValidator.java
>  1ab5be35 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerImpl.java
>  86ff0cc2 
> 
> 
> Diff: https://reviews.apache.org/r/66590/diff/4/
> 
> 
> Testing
> ---
> 
> full build,added unit tests, tested code on a cluster.
> 
> 
> Thanks,
> 
> Steve Moist
> 
>

Re: Review Request 66590: SENTRY-2201 Initial Attriubte based access control

[Steve Moist via Review Board]

> On April 19, 2018, 3:23 p.m., Arjun Mishra wrote:
> > sentry-abac/src/main/java/org/apache/sentry/abac/Attribute.java
> > Lines 19 (patched)
> > 
> >
> > Is Attribute persisted?

Currently, maybe.  I need to update the doc that mentions it as we've had some 
discussions about this part.  But it would be stored in a cache that then could 
be persisted to disk.  Or even stored in the Sentry DB.


> On April 19, 2018, 3:23 p.m., Arjun Mishra wrote:
> > sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java
> > Lines 44 (patched)
> > 
> >
> > If Attribute and SentryObject are tables, I believe that this should 
> > also be a table. Seems like it is a Many-To-Many mapping. If you had the 
> > database we won't need many of the below methods right?

See comment above.


> On April 19, 2018, 3:23 p.m., Arjun Mishra wrote:
> > sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java
> > Lines 60 (patched)
> > 
> >
> > Thinking if there is a better way to do this
> > 
> > Instead of addEntry, can we have addAttributeToObject, and 
> > addObjectToAttribute? Then we could have 2 methods as opposed to 3 separate 
> > addEntry methods

For static attribute ingestion (snapshots) we'd need the addEntry(Attribute, 
SentryOBject) method.  For the deltas, I think we'd need the individual 
methods, but renaming them would be beneficial.  I'll make that change.


> On April 19, 2018, 3:23 p.m., Arjun Mishra wrote:
> > sentry-abac/src/main/java/org/apache/sentry/abac/SentryObject.java
> > Lines 27 (patched)
> > 
> >
> > this class seems identical to Attrubute. Any reason why we have another 
> > one? Is this going to be persisted?

This class is planned to be expanded upon later on.  We have an additional 
field contentDescriptor that is planned to be used here.  It allows users to 
classify fields such as "credit_card", "ssn", etc.  The contentDescriptor 
information and other related information needs to be persisted eventually or 
cached depending on the above cache/persistance statement.


- Steve


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


On April 16, 2018, 7:51 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66590/
> ---
> 
> (Updated April 16, 2018, 7:51 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> This is the inital draft of attribute based access control.
> 
> 
> Diffs
> -
> 
>   pom.xml 16a3838a 
>   sentry-abac/example-definition.json PRE-CREATION 
>   sentry-abac/pom.xml PRE-CREATION 
>   sentry-abac/src/main/java/org/apache/sentry/abac/Attribute.java 
> PRE-CREATION 
>   sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java 
> PRE-CREATION 
>   sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapAdapter.java 
> PRE-CREATION 
>   
> sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapKeyException.java
>  PRE-CREATION 
>   
> sentry-abac/src/main/java/org/apache/sentry/abac/GenericAttributeProvider.java
>  PRE-CREATION 
>   
> sentry-abac/src/main/java/org/apache/sentry/abac/SentryAttributeAuthorizer.java
>  PRE-CREATION 
>   sentry-abac/src/main/java/org/apache/sentry/abac/SentryObject.java 
> PRE-CREATION 
>   
> sentry-abac/src/main/java/org/apache/sentry/abac/StaticAttributeProvider.java 
> PRE-CREATION 
>   sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttribute.java 
> PRE-CREATION 
>   
> sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttributeMap.java 
> PRE-CREATION 
>   
> sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestSentryAttributeAuthorizer.java
>  PRE-CREATION 
>   
> sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestStaticProvider.java
>  PRE-CREATION 
>   sentry-abac/src/test/resources/abac.props PRE-CREATION 
>   sentry-binding/sentry-binding-hive/pom.xml ccfa9cfe 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryValidator.java
>  1ab5be35 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerImpl.java
>  86ff0cc2 
> 
> 
> Diff: https://reviews.apache.org/r/66590/diff/4/
> 
> 
> Testing
> ---
> 
> full build,added unit tests, tested code on a cluster.
> 
> 
> Thanks,
> 
> Steve Moist
> 
>

Re: Review Request 66590: SENTRY-2201 Initial Attriubte based access control

[Steve Moist via Review Board]

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

(Updated April 16, 2018, 7:51 p.m.)


Review request for sentry.


Repository: sentry


Description
---

This is the inital draft of attribute based access control.


Diffs (updated)
-

  pom.xml 16a3838a 
  sentry-abac/example-definition.json PRE-CREATION 
  sentry-abac/pom.xml PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/Attribute.java PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java 
PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapAdapter.java 
PRE-CREATION 
  
sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapKeyException.java 
PRE-CREATION 
  
sentry-abac/src/main/java/org/apache/sentry/abac/GenericAttributeProvider.java 
PRE-CREATION 
  
sentry-abac/src/main/java/org/apache/sentry/abac/SentryAttributeAuthorizer.java 
PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/SentryObject.java 
PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/StaticAttributeProvider.java 
PRE-CREATION 
  sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttribute.java 
PRE-CREATION 
  sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttributeMap.java 
PRE-CREATION 
  
sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestSentryAttributeAuthorizer.java
 PRE-CREATION 
  
sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestStaticProvider.java 
PRE-CREATION 
  sentry-abac/src/test/resources/abac.props PRE-CREATION 
  sentry-binding/sentry-binding-hive/pom.xml ccfa9cfe 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryValidator.java
 1ab5be35 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerImpl.java
 86ff0cc2 


Diff: https://reviews.apache.org/r/66590/diff/4/

Changes: https://reviews.apache.org/r/66590/diff/3-4/


Testing
---

full build,added unit tests, tested code on a cluster.


Thanks,

Steve Moist

Re: Review Request 66590: SENTRY-2201 Initial Attriubte based access control

[Steve Moist via Review Board]

> On April 13, 2018, 9:13 p.m., Anthony Young-Garner wrote:
> > sentry-abac/notes.txt
> > Lines 19 (patched)
> > 
> >
> > Lines 19 - 20 can be removed if the example-delta.json file is removed.

I just deleted the notes file.


> On April 13, 2018, 9:13 p.m., Anthony Young-Garner wrote:
> > sentry-abac/pom.xml
> > Lines 25 (patched)
> > 
> >
> > Should the project version be parameterized?

None of the other modules when declaring it's parent are parameterized.


> On April 13, 2018, 9:13 p.m., Anthony Young-Garner wrote:
> > sentry-abac/src/main/java/org/apache/sentry/abac/SentryObject.java
> > Lines 33 (patched)
> > 
> >
> > Add field, getter and setter for descriptor (contentDescriptor). Also 
> > add a two-arg constructor and update toString, equals and hashCode methods.

That's a moderate size change to make at this point, it would be better to 
include it in another ticket.  Especially since we have profiles and actions to 
include as well.


- Steve


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


On April 12, 2018, 8:45 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66590/
> ---
> 
> (Updated April 12, 2018, 8:45 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> This is the inital draft of attribute based access control.
> 
> 
> Diffs
> -
> 
>   pom.xml 16a3838a 
>   sentry-abac/example-definition.json PRE-CREATION 
>   sentry-abac/example-delta.json PRE-CREATION 
>   sentry-abac/notes.txt PRE-CREATION 
>   sentry-abac/pom.xml PRE-CREATION 
>   sentry-abac/src/main/java/org/apache/sentry/abac/Attribute.java 
> PRE-CREATION 
>   sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java 
> PRE-CREATION 
>   sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapAdapter.java 
> PRE-CREATION 
>   
> sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapKeyException.java
>  PRE-CREATION 
>   
> sentry-abac/src/main/java/org/apache/sentry/abac/GenericAttributeProvider.java
>  PRE-CREATION 
>   
> sentry-abac/src/main/java/org/apache/sentry/abac/SentryAttributeAuthorizer.java
>  PRE-CREATION 
>   sentry-abac/src/main/java/org/apache/sentry/abac/SentryObject.java 
> PRE-CREATION 
>   
> sentry-abac/src/main/java/org/apache/sentry/abac/StaticAttributeProvider.java 
> PRE-CREATION 
>   sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttribute.java 
> PRE-CREATION 
>   
> sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttributeMap.java 
> PRE-CREATION 
>   
> sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestSentryAttributeAuthorizer.java
>  PRE-CREATION 
>   
> sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestStaticProvider.java
>  PRE-CREATION 
>   sentry-abac/src/test/resources/abac.props PRE-CREATION 
>   sentry-binding/sentry-binding-hive/pom.xml ccfa9cfe 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryValidator.java
>  1ab5be35 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerImpl.java
>  86ff0cc2 
> 
> 
> Diff: https://reviews.apache.org/r/66590/diff/3/
> 
> 
> Testing
> ---
> 
> full build,added unit tests, tested code on a cluster.
> 
> 
> Thanks,
> 
> Steve Moist
> 
>

Re: Review Request 66590: SENTRY-2201 Initial Attriubte based access control

[Steve Moist via Review Board]

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

(Updated April 12, 2018, 8:45 p.m.)


Review request for sentry.


Changes
---

Adding in actual abac enforcement.


Repository: sentry


Description
---

This is the inital draft of attribute based access control.


Diffs (updated)
-

  pom.xml 16a3838a 
  sentry-abac/example-definition.json PRE-CREATION 
  sentry-abac/example-delta.json PRE-CREATION 
  sentry-abac/notes.txt PRE-CREATION 
  sentry-abac/pom.xml PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/Attribute.java PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java 
PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapAdapter.java 
PRE-CREATION 
  
sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapKeyException.java 
PRE-CREATION 
  
sentry-abac/src/main/java/org/apache/sentry/abac/GenericAttributeProvider.java 
PRE-CREATION 
  
sentry-abac/src/main/java/org/apache/sentry/abac/SentryAttributeAuthorizer.java 
PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/SentryObject.java 
PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/StaticAttributeProvider.java 
PRE-CREATION 
  sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttribute.java 
PRE-CREATION 
  sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttributeMap.java 
PRE-CREATION 
  
sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestSentryAttributeAuthorizer.java
 PRE-CREATION 
  
sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestStaticProvider.java 
PRE-CREATION 
  sentry-abac/src/test/resources/abac.props PRE-CREATION 
  sentry-binding/sentry-binding-hive/pom.xml ccfa9cfe 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryValidator.java
 1ab5be35 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerImpl.java
 86ff0cc2 


Diff: https://reviews.apache.org/r/66590/diff/3/

Changes: https://reviews.apache.org/r/66590/diff/2-3/


Testing
---

full build,added unit tests, tested code on a cluster.


Thanks,

Steve Moist

Re: Review Request 66590: SENTRY-2201 Initial Attriubte based access control

[Steve Moist via Review Board]

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

(Updated April 12, 2018, 8:45 p.m.)


Review request for sentry.


Changes
---

Adding changes commented on https://reviews.apache.org/r/66480/


Repository: sentry


Description
---

This is the inital draft of attribute based access control.


Diffs (updated)
-

  pom.xml 16a3838a 
  sentry-abac/example-definition.json PRE-CREATION 
  sentry-abac/example-delta.json PRE-CREATION 
  sentry-abac/notes.txt PRE-CREATION 
  sentry-abac/pom.xml PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/Attribute.java PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java 
PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapAdapter.java 
PRE-CREATION 
  
sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapKeyException.java 
PRE-CREATION 
  
sentry-abac/src/main/java/org/apache/sentry/abac/GenericAttributeProvider.java 
PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/SentryObject.java 
PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/StaticAttributeProvider.java 
PRE-CREATION 
  sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttribute.java 
PRE-CREATION 
  sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttributeMap.java 
PRE-CREATION 
  
sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestStaticProvider.java 
PRE-CREATION 


Diff: https://reviews.apache.org/r/66590/diff/2/

Changes: https://reviews.apache.org/r/66590/diff/1-2/


Testing
---

full build,added unit tests, tested code on a cluster.


Thanks,

Steve Moist

Review Request 66590: SENTRY-2201 Initial Attriubte based access control

[Steve Moist via Review Board]

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

Review request for sentry.


Repository: sentry


Description
---

This is the inital draft of attribute based access control.


Diffs
-

  pom.xml 61e0f970017aa8ddf38a59c7c1334fadb97abe40 
  sentry-abac/example-definition.json PRE-CREATION 
  sentry-abac/example-delta.json PRE-CREATION 
  sentry-abac/notes.txt PRE-CREATION 
  sentry-abac/pom.xml PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/Attribute.java PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java 
PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapAdapter.java 
PRE-CREATION 
  
sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapKeyException.java 
PRE-CREATION 
  
sentry-abac/src/main/java/org/apache/sentry/abac/GenericAttributeProvider.java 
PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/SentryObject.java 
PRE-CREATION 
  sentry-abac/src/main/java/org/apache/sentry/abac/StaticAttributeProvider.java 
PRE-CREATION 
  sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttribute.java 
PRE-CREATION 
  sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttributeMap.java 
PRE-CREATION 
  
sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestStaticProvider.java 
PRE-CREATION 


Diff: https://reviews.apache.org/r/66590/diff/1/


Testing
---

full build,added unit tests, tested code on a cluster.


Thanks,

Steve Moist

Re: Review Request 66480: SENTRY-2140: Static Attribute Ingestion

[Steve Moist via Review Board]

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




sentry-abac/notes.txt
Lines 23 (patched)


Remove this.



sentry-abac/src/main/java/org/apache/sentry/abac/Attribute.java
Lines 27 (patched)


Remove navigator mentions.



sentry-abac/src/main/java/org/apache/sentry/abac/StaticAttributeProvider.java
Lines 81 (patched)


Deal with the different exception cases here.

File exists but not correct permissions.



sentry-abac/src/main/java/org/apache/sentry/abac/StaticAttributeProvider.java
Lines 82 (patched)


Again convert to FileUtils read into string.



sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttributeMap.java
Lines 331 (patched)


Add if exists?



sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttributeMap.java
Lines 333 (patched)


Again convert to FileUtils.



sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttributeMap.java
Lines 362 (patched)


Use logger



sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttributeMap.java
Lines 371 (patched)


Convert to FileUtils. readFileToString



sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttributeMap.java
Lines 377 (patched)


Conver to logger.info/debug



sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestStaticProvider.java
Lines 69 (patched)


File Utils


- Steve Moist


On April 5, 2018, 9:20 p.m., Liam Sargent wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66480/
> ---
> 
> (Updated April 5, 2018, 9:20 p.m.)
> 
> 
> Review request for sentry, Na Li, Steve Moist, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2189
> https://issues.apache.org/jira/browse/SENTRY-2189
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> - Example configuration file: example-definition.json
> - Attribute: plain old java object for string attribute.
> - Object: plain old java object for fully qualified sentry object.
> - AttributeMap: Bidirectional map type for quick lookup between Attribute -> 
> Object, Object -> Attribute.
> - AttributeMapAdapter: JSON deserializer for file-based attribute ingestion.
> 
> 
> Diffs
> -
> 
>   pom.xml 61e0f970017aa8ddf38a59c7c1334fadb97abe40 
>   sentry-abac/example-definition.json PRE-CREATION 
>   sentry-abac/example-delta.json PRE-CREATION 
>   sentry-abac/notes.txt PRE-CREATION 
>   sentry-abac/pom.xml PRE-CREATION 
>   sentry-abac/src/main/java/org/apache/sentry/abac/Attribute.java 
> PRE-CREATION 
>   sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java 
> PRE-CREATION 
>   sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapAdapter.java 
> PRE-CREATION 
>   
> sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapKeyException.java
>  PRE-CREATION 
>   
> sentry-abac/src/main/java/org/apache/sentry/abac/GenericAttributeProvider.java
>  PRE-CREATION 
>   sentry-abac/src/main/java/org/apache/sentry/abac/SentryObject.java 
> PRE-CREATION 
>   
> sentry-abac/src/main/java/org/apache/sentry/abac/StaticAttributeProvider.java 
> PRE-CREATION 
>   sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttribute.java 
> PRE-CREATION 
>   
> sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttributeMap.java 
> PRE-CREATION 
>   
> sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestStaticProvider.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66480/diff/1/
> 
> 
> Testing
> ---
> 
> - Unittests for all useful functions.
> 
> 
> Thanks,
> 
> Liam Sargent
> 
>

Re: Review Request 66360: SENTRY-2192: supress date value in @Generated annotation generated by thrift

[Steve Moist via Review Board]

> On March 29, 2018, 5:29 p.m., Steve Moist wrote:
> > Tested this out with some by adding the thrift apis I wrote (new endpoints 
> > and objects).  It generates only the necessary files and changes in most 
> > cases.  When adding new endpoints and objects it shifts things around in 
> > the a common generated file.  This is fine but might make reviews a little 
> > annoying.
> > 
> > Look at the maven plugin in the code I submitted for the associated ticket. 
> >  It's cleaner than the custom mvn build stuff that exists now, see if that 
> > can be replaced.
> 
> kalyan kumar kalvagadda wrote:
> Steve,
> 
> Can you provide more details on the maven plug-in you are talking about.

See the diff/review for SENTRY-2188 It contains that in the pom.


- Steve


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


On March 29, 2018, 1:16 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66360/
> ---
> 
> (Updated March 29, 2018, 1:16 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Na Li, Steve Moist, and Sergio 
> Pena.
> 
> 
> Bugs: SENTRY-2192
> https://issues.apache.org/jira/browse/SENTRY-2192
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> With the current thrift options used, @Generated annotation adds date which 
> kind of updates all the thrift generated files when --thriftif option used.
> 
> When someone makes some changes to any of the thrift definitions and tries to 
> generate the source all the auto generated files get updated.
> 
> This can be avoided by suppressing date in the @generated annotation.
> 
> 
> Diffs
> -
> 
>   sentry-hdfs/sentry-hdfs-common/pom.xml 
> 5c6c96c46ee384ba5981611333741fde3bd59e10 
>   
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/SentryHDFSService.java
>  ac1fa6af17239e23557614e7149c7cc911e28e12 
>   
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TAuthzUpdateRequest.java
>  62fd0bb2426db427241a5f805c9e250b306b4947 
>   
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TAuthzUpdateResponse.java
>  68aec801ae7f38f8cbbe8549f5d9df79b609b114 
>   
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathChanges.java
>  8d3297ffe6f2de4dc3dc82f82a70b5ad2a235911 
>   
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathEntry.java
>  210d2191c38603d4a951a590eee85c8fccdde124 
>   
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathsDump.java
>  df5b7b10656102c6471885cfea73b0ed8dd67dd7 
>   
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathsUpdate.java
>  9fd3924f138696707df4ab1bb5c994005b9bd1f4 
>   
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPermissionsUpdate.java
>  e8c09322ac6f8b8210bc4dd8869ad3b0353b63d9 
>   
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPrivilegeChanges.java
>  0472f33ec49652ef402bcb332321fb2803b923d9 
>   
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TRoleChanges.java
>  1a502080896b2b85e9bf6ae4780b4a2db35473f7 
>   sentry-provider/sentry-provider-db/pom.xml 
> 4751549bcc40d60ca8f5400dd371761bc6fbf2d5 
>   
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyService.java
>  ae4db6a06c72a93e62bc5a3c2a17776bcb7d 
>   
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleAddGroupsRequest.java
>  0b62dae0588859bceea54b8f4b49689561a7a4f9 
>   
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleAddGroupsResponse.java
>  29e5baf8d3ecd67ae655f513eaba5a1a2bc14b9a 
>   
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleDeleteGroupsRequest.java
>  42293b7bbda879d87f69721cfc3f4fd533055ae2 
>   
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleDeleteGroupsResponse.java
>  a3a660cc9d53411588c5a681b84a75a35ebc3f96 
>   
> 

Re: Review Request 66360: SENTRY-2192: supress date value in @Generated annotation generated by thrift

[Steve Moist via Review Board]

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



Tested this out with some by adding the thrift apis I wrote (new endpoints and 
objects).  It generates only the necessary files and changes in most cases.  
When adding new endpoints and objects it shifts things around in the a common 
generated file.  This is fine but might make reviews a little annoying.

Look at the maven plugin in the code I submitted for the associated ticket.  
It's cleaner than the custom mvn build stuff that exists now, see if that can 
be replaced.

- Steve Moist


On March 29, 2018, 1:16 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66360/
> ---
> 
> (Updated March 29, 2018, 1:16 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Na Li, Steve Moist, and Sergio 
> Pena.
> 
> 
> Bugs: SENTRY-2192
> https://issues.apache.org/jira/browse/SENTRY-2192
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> With the current thrift options used, @Generated annotation adds date which 
> kind of updates all the thrift generated files when --thriftif option used.
> 
> When someone makes some changes to any of the thrift definitions and tries to 
> generate the source all the auto generated files get updated.
> 
> This can be avoided by suppressing date in the @generated annotation.
> 
> 
> Diffs
> -
> 
>   sentry-hdfs/sentry-hdfs-common/pom.xml 
> 5c6c96c46ee384ba5981611333741fde3bd59e10 
>   
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/SentryHDFSService.java
>  ac1fa6af17239e23557614e7149c7cc911e28e12 
>   
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TAuthzUpdateRequest.java
>  62fd0bb2426db427241a5f805c9e250b306b4947 
>   
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TAuthzUpdateResponse.java
>  68aec801ae7f38f8cbbe8549f5d9df79b609b114 
>   
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathChanges.java
>  8d3297ffe6f2de4dc3dc82f82a70b5ad2a235911 
>   
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathEntry.java
>  210d2191c38603d4a951a590eee85c8fccdde124 
>   
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathsDump.java
>  df5b7b10656102c6471885cfea73b0ed8dd67dd7 
>   
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathsUpdate.java
>  9fd3924f138696707df4ab1bb5c994005b9bd1f4 
>   
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPermissionsUpdate.java
>  e8c09322ac6f8b8210bc4dd8869ad3b0353b63d9 
>   
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPrivilegeChanges.java
>  0472f33ec49652ef402bcb332321fb2803b923d9 
>   
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TRoleChanges.java
>  1a502080896b2b85e9bf6ae4780b4a2db35473f7 
>   sentry-provider/sentry-provider-db/pom.xml 
> 4751549bcc40d60ca8f5400dd371761bc6fbf2d5 
>   
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyService.java
>  ae4db6a06c72a93e62bc5a3c2a17776bcb7d 
>   
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleAddGroupsRequest.java
>  0b62dae0588859bceea54b8f4b49689561a7a4f9 
>   
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleAddGroupsResponse.java
>  29e5baf8d3ecd67ae655f513eaba5a1a2bc14b9a 
>   
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleDeleteGroupsRequest.java
>  42293b7bbda879d87f69721cfc3f4fd533055ae2 
>   
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleDeleteGroupsResponse.java
>  a3a660cc9d53411588c5a681b84a75a35ebc3f96 
>   
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleGrantPrivilegeRequest.java
>  49cfe4b0057721839c442353b6441013d5d6438c 
>   
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleGrantPrivilegeResponse.java
>  

Re: Review Request 66336: SENTRY-2190: Have verbose debug logs in CounterWait class

[Steve Moist via Review Board]

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


Ship it!




Ship It!

- Steve Moist


On March 28, 2018, 4:14 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66336/
> ---
> 
> (Updated March 28, 2018, 4:14 p.m.)
> 
> 
> Review request for sentry and Na Li.
> 
> 
> Bugs: SENTRY-2190
> https://issues.apache.org/jira/browse/SENTRY-2190
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Debug logging in CounterWait is not good enough to actually an issue around 
> this area. It's a good idea to extend the current logging.
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/CounterWait.java
>  d8c82970b56b1599a07f0e26edab8ed3d59b9948 
> 
> 
> Diff: https://reviews.apache.org/r/66336/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Review Request 66319: 75+ files are uneccessarily changed when generetating new Thrift files.

[Steve Moist via Review Board]

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

Review request for sentry.


Repository: sentry


Description
---

Changed the pom to use a different plugin to generate thrift files rather than 
the custom made one that sentry uses.  Changed the structure of where thrift 
gets generated to and also removed the generated files from git.


Diffs
-

  sentry-hdfs/sentry-hdfs-common/pom.xml 5c6c96c4 
  
sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/SentryHDFSService.java
 ac1fa6af 
  
sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TAuthzUpdateRequest.java
 62fd0bb2 
  
sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TAuthzUpdateResponse.java
 68aec801 
  
sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathChanges.java
 8d3297ff 
  
sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathEntry.java
 210d2191 
  
sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathsDump.java
 df5b7b10 
  
sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathsUpdate.java
 9fd3924f 
  
sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPermissionsUpdate.java
 e8c09322 
  
sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPrivilegeChanges.java
 0472f33e 
  
sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TRoleChanges.java
 1a502080 
  
sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/sentry_hdfs_serviceConstants.java
 a7fd7124 
  sentry-provider/sentry-provider-db/pom.xml 4751549b 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyService.java
 ae4d 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleAddGroupsRequest.java
 0b62dae0 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleAddGroupsResponse.java
 29e5baf8 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleDeleteGroupsRequest.java
 42293b7b 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleDeleteGroupsResponse.java
 a3a660cc 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleGrantPrivilegeRequest.java
 49cfe4b0 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleGrantPrivilegeResponse.java
 6890337d 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleRevokePrivilegeRequest.java
 91793fee 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAlterSentryRoleRevokePrivilegeResponse.java
 bd7127ba 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TAuthorizable.java
 6ab06f70 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TCreateSentryRoleRequest.java
 786c6437 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TCreateSentryRoleResponse.java
 b2c14a03 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TDropPrivilegesRequest.java
 6258969f 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TDropPrivilegesResponse.java
 8d0ecaf7 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TDropSentryRoleRequest.java
 1eaf496d 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TDropSentryRoleResponse.java
 61f7c4d2 
  
sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/generic/service/thrift/TListSentryPrivilegesByAuthRequest.java
 ff51de7d 
  

Re: Review Request 65639: SENTRY-1242 Enable getting all privileges on a hive object

[Steve Moist via Review Board]

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

(Updated March 9, 2018, 6:40 p.m.)


Review request for sentry.


Repository: sentry


Description
---

Enabled use of show grant on table and show grant on database for the current 
user.


Diffs (updated)
-

  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java
 23246c90 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java
 3ac49fa6 
  
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java
 2e3fd7f3 


Diff: https://reviews.apache.org/r/65639/diff/3/

Changes: https://reviews.apache.org/r/65639/diff/2-3/


Testing
---

Added unit tests, mvn clean package on sentry-binding/sentry-binding-hive


Thanks,

Steve Moist

Re: Review Request 65953: SENTRY-2165: NotificationProcesser process notification methods have logs wrongly flagged as ERROR

[Steve Moist via Review Board]

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




sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java
Line 403 (original), 403 (patched)


The rest are warn, why debug?


- Steve Moist


On March 8, 2018, 2:39 p.m., Arjun Mishra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65953/
> ---
> 
> (Updated March 8, 2018, 2:39 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda, Na Li, Steve Moist, and 
> Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> "Alter table event has incomplete information" and "Alter table notification 
> ignored as neither name nor location has changed" are logged as ERROR. This 
> can flood the logs and be a distraction to analyzing actual issues. These 
> should be set at INFO level
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java
>  94a0b0f12 
> 
> 
> Diff: https://reviews.apache.org/r/65953/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 65945: SENTRY-2164: Convert uses of TransactionBlock to lambdas

[Steve Moist via Review Board]

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


Ship it!




Ship It!

- Steve Moist


On March 7, 2018, 7:14 a.m., Alexander Kolbasov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65945/
> ---
> 
> (Updated March 7, 2018, 7:14 a.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2164
> https://issues.apache.org/jira/browse/SENTRY-2164
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-2164: Convert uses of TransactionBlock to lambdas
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
>  4cb46abc52985b6af65ef06e4c14ae4bf92bec0c 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  7a31a016f6867284446f685e59b1377ae44e970b 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionBlock.java
>  c7b19cee9e5819bea56debcb5091d92a4cba9019 
> 
> 
> Diff: https://reviews.apache.org/r/65945/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>

Re: Review Request 65953: SENTRY-2165: NotificationProcesser processAlterTable method has logs wrongly flagged as ERROR when they should be INFO

[Steve Moist via Review Board]

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




sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java
Line 388 (original), 388 (patched)


Description says it should be info, why is it warn?


- Steve Moist


On March 7, 2018, 5:03 p.m., Arjun Mishra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65953/
> ---
> 
> (Updated March 7, 2018, 5:03 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> "Alter table event has incomplete information" and "Alter table notification 
> ignored as neither name nor location has changed" are logged as ERROR. This 
> can flood the logs and be a distraction to analyzing actual issues. These 
> should be set at INFO level
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java
>  94a0b0f12 
> 
> 
> Diff: https://reviews.apache.org/r/65953/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 65866: SENTRY-2146: Add better error handling to ResourceAuthorizationProvider and improve logging in related classes

[Steve Moist via Review Board]

> On March 1, 2018, 5:39 p.m., Steve Moist wrote:
> > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java
> > Lines 140 (patched)
> > 
> >
> > I don't find stating illegargumentexception to be helpful.  What kind 
> > of illegal argument woould trigger this exception to happen?
> 
> Arjun Mishra wrote:
> See KeyValue class line 37

I mean more express the exception message in a more user friendly way.  What is 
it trying to accomplish here, and why are we specifically logging it as a IAE 
when we could just collapse all the messages into single catch(Exception e) and 
log it there.


- Steve


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


On March 1, 2018, 6:19 p.m., Arjun Mishra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65866/
> ---
> 
> (Updated March 1, 2018, 6:19 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Brian Towles, kalyan kumar 
> kalvagadda, Liam Sargent, Na Li, Steve Moist, Sergio Pena, Vadim Spector, and 
> Xinran Tinney.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> There are a bunch of improvements that should be made to 
> ResourceAuthorizationProvider. For example, exceptions thrown by 
> privilegeFactory.createPrivilege are not gracefully handled. Makes debugging 
> hard. 
> 
> We also need to add a lot more logging that needs to be added to related 
> classes
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
>  7565a34b5 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
>  09bd9b566 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java
>  447deaf58 
>   
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/KeyValue.java
>  4e944e5f2 
>   
> sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java
>  ab5560994 
>   
> sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/PrivilegeFactory.java
>  2f8296bfa 
>   
> sentry-policy/sentry-policy-engine/src/main/java/org/apache/sentry/policy/engine/common/CommonPrivilegeFactory.java
>  d338f0edd 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java
>  a9b98f319 
> 
> 
> Diff: https://reviews.apache.org/r/65866/diff/2/
> 
> 
> Testing
> ---
> 
> mvn -f sentry-binding/sentry-binding-hive/pom.xml test
> mvn -f sentry-core/sentry-core-common/pom.xml test
> mvn -f sentry-policy/sentry-policy-common/pom.xml test
> mvn -f sentry-policy/sentry-policy-engine/pom.xml test
> mvn -f sentry-provider/sentry-provider-common/pom.xml test
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 65639: SENTRY-1242 Enable getting all privileges on a hive object

[Steve Moist via Review Board]

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

(Updated March 5, 2018, 9:40 p.m.)


Review request for sentry.


Repository: sentry


Description
---

Enabled use of show grant on table and show grant on database for the current 
user.


Diffs (updated)
-

  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java
 23246c90 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java
 3ac49fa6 
  
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java
 2e3fd7f3 


Diff: https://reviews.apache.org/r/65639/diff/2/

Changes: https://reviews.apache.org/r/65639/diff/1-2/


Testing
---

Added unit tests, mvn clean package on sentry-binding/sentry-binding-hive


Thanks,

Steve Moist

Re: Review Request 65867: SENTRY-2147 - Fix Javadoc for SentryHiveAuthorizerFactory

[Steve Moist via Review Board]

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


Ship it!




Ship It!

- Steve Moist


On March 5, 2018, 5:25 p.m., Colm O hEigeartaigh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65867/
> ---
> 
> (Updated March 5, 2018, 5:25 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-2147
> https://issues.apache.org/jira/browse/SENTRY-2147
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> The Javadoc for SentryHiveAuthorizerFactory incorrectly states that it should 
> be configured as follows:
> 
>  
>hive.security.authorization.enable
>   
> org.apache.sentry.binding.hive.authz.SentryHiveAuthorizerFactory
> 
> 
> Instead it should be "hive.security.authorization.manager".
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerFactory.java
>  db8e4288 
> 
> 
> Diff: https://reviews.apache.org/r/65867/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Colm O hEigeartaigh
> 
>

Re: Review Request 65816: SENTRY-2145 - Some misc code cleanups

[Steve Moist via Review Board]

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




sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/authorizer/DefaultSentryValidator.java
Line 217 (original), 217 (patched)


Insert String permsRequired = permsRequired.toString(); here.  The 
toString() creates a new string everytime this is called.


- Steve Moist


On Feb. 27, 2018, 2:45 p.m., Colm O hEigeartaigh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65816/
> ---
> 
> (Updated Feb. 27, 2018, 2:45 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-2145
> https://issues.apache.org/jira/browse/SENTRY-2145
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> This task is to implement some misc code cleanups:
> 
> a) Use StringBuilder instead of looping and using "+" with a String
> 
> b) Use EntrySet instead of KeySet when the value is also required
> 
> c) Make sure constants are static final.
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/authorizer/DefaultSentryValidator.java
>  6c2410b0 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryValidator.java
>  319a1be5 
>   
> sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java
>  908c80f1 
>   
> sentry-core/sentry-core-model-solr/src/main/java/org/apache/sentry/core/model/solr/AdminOperation.java
>  c054b7dc 
>   sentry-dist/src/license/THIRD-PARTY.properties 7226acdf 
> 
> 
> Diff: https://reviews.apache.org/r/65816/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Colm O hEigeartaigh
> 
>

Re: Review Request 65704: SENTRY-2143: Table renames should synchronize with Sentry

[Steve Moist via Review Board]

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




sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java
Lines 141 (patched)


Possible NPE here, move this if up before you do Table oldTable = 
tableEvent.getOldTable();
Table newTable = tableEvent.getNewTable();



sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java
Lines 157 (patched)


What if any of these strings are empty?



sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java
Lines 239 (patched)


Nit: Please fix the spacing with the new line here, it's confusing when 
concatenting strings.


- Steve Moist


On Feb. 22, 2018, 6:55 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65704/
> ---
> 
> (Updated Feb. 22, 2018, 6:55 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and 
> Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> wait for HMS sync at alter table, which including table rename and changing 
> table columns
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java
>  24d7763 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  7a31a01 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java
>  5fe6625 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
>  9b0aeb2 
> 
> 
> Diff: https://reviews.apache.org/r/65704/diff/4/
> 
> 
> Testing
> ---
> 
> unit tests succeeded
> 
> 
> Thanks,
> 
> Na Li
> 
>

Re: Review Request 65642: SENTRY-2141: Sentry Privilege TimeStamp is not converted to grantTime in HivePrivilegeInfo correctly

[Steve Moist via Review Board]

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


Ship it!




Ship It!

- Steve Moist


On Feb. 14, 2018, 12:51 a.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65642/
> ---
> 
> (Updated Feb. 14, 2018, 12:51 a.m.)
> 
> 
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio 
> Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> sentry CreateTime is the difference, measured in milliseconds, between the 
> current time and midnight, January 1, 1970 UTC. hive granttime is in seconds. 
> So need to convert the time.
> 
> The original code just cost the timestamp from long to int without converting 
> milliseconds to seconds. Therefore, the timestamp value is wrong when 
> retrieving the privilege from hive.
> 
> The solution is to convert the time correctly.
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/util/SentryAuthorizerUtil.java
>  b536283 
> 
> 
> Diff: https://reviews.apache.org/r/65642/diff/1/
> 
> 
> Testing
> ---
> 
> tested manually by stepping into code. I cannot test it in unit test because 
> Hive returns timestamp as -1L for test mode
> 
>   DDLTask.writeGrantInfo
> static String writeGrantInfo(List privileges, boolean 
> testMode) {
> if (privileges != null && !privileges.isEmpty()) {
>   StringBuilder builder = new StringBuilder();
>   Collections.sort(privileges, new Comparator() {
> public int compare(HivePrivilegeInfo o1, HivePrivilegeInfo o2) {
>   int compare = o1.getObject().compareTo(o2.getObject());
>   if (compare == 0) {
> compare = o1.getPrincipal().compareTo(o2.getPrincipal());
>   }
> 
>   if (compare == 0) {
> compare = o1.getPrivilege().compareTo(o2.getPrivilege());
>   }
> 
>   return compare;
> }
>   });
>   Iterator var3 = privileges.iterator();
> 
>   while(var3.hasNext()) {
> HivePrivilegeInfo privilege = (HivePrivilegeInfo)var3.next();
> HivePrincipal principal = privilege.getPrincipal();
> HivePrivilegeObject resource = privilege.getObject();
> HivePrincipal grantor = privilege.getGrantorPrincipal();
> appendNonNull(builder, resource.getDbname(), true);
> appendNonNull(builder, resource.getObjectName());
> appendNonNull(builder, resource.getPartKeys());
> appendNonNull(builder, resource.getColumns());
> appendNonNull(builder, principal.getName());
> appendNonNull(builder, principal.getType());
> appendNonNull(builder, privilege.getPrivilege().getName());
> appendNonNull(builder, privilege.isGrantOption());
> appendNonNull(builder, testMode ? -1L : 
> (long)privilege.getGrantTime() * 1000L); <-- in test mode, does not 
> return real timestamp
> appendNonNull(builder, grantor.getName());
>   }
> 
>   return builder.toString();
> } else {
>   return "";
> }
>   }
> 
> 
> Thanks,
> 
> Na Li
> 
>

Re: Review Request 65641: SENTRY-853 Handle show grant on failure correctly

[Steve Moist via Review Board]

> On Feb. 13, 2018, 10:51 p.m., Sergio Pena wrote:
> > sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java
> > Line 363 (original), 363 (patched)
> > 
> >
> > I'm confused about this. Don't we support 'show grant role' already? 
> > Why is Sentry displaying that it does not support show grant on role?

We do.  This is "show role grant role ".  It looks like it has never been 
supported, the only show role grant supported is "show role grant group 
".  The hive docs make mention to "show role grant role" 
https://cwiki.apache.org/confluence/display/Hive/SQL+Standard+Based+Hive+Authorization#SQLStandardBasedHiveAuthorization-RoleManagementCommands
 but does not include an example of the execution.


- Steve


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


On Feb. 13, 2018, 10:35 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65641/
> ---
> 
> (Updated Feb. 13, 2018, 10:35 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Improved the error message for when a show command is run instead of a 
> generic message that is used improperly.
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/hadoop/hive/SentryHiveConstants.java
>  38d1f468 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java
>  1e520c0b 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java
>  14a96191 
>   
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java
>  c75f57d6 
> 
> 
> Diff: https://reviews.apache.org/r/65641/diff/1/
> 
> 
> Testing
> ---
> 
> Setup cluster and ran commands through beeline:
> 0: jdbc:hive2://server> show grant user usercomedy;
> Error: Error while compiling statement: FAILED: SemanticException Sentry does 
> not allow privileges to be shown for: USER (state=42000,code=4)
> 0: jdbc:hive2://server> show grant on table movies;
> Error: Error while compiling statement: FAILED: SemanticException Sentry does 
> not allow privileges to be shown for: USER (state=42000,code=4)
> 0: jdbc:hive2://server> show grant on database moviesdb;
> Error: Error while compiling statement: FAILED: SemanticException Sentry does 
> not allow privileges to be shown for: USER (state=42000,code=4)
> 0: jdbc:hive2://server> show grant group comedy_group;
> Error: Error while compiling statement: FAILED: SemanticException Sentry does 
> not allow privileges to be shown for: GROUP (state=42000,code=4)
> 0: jdbc:hive2://server> show role grant role comedyrole;
> Error: Error while compiling statement: FAILED: SemanticException Sentry does 
> not allow privileges to be shown for: ROLE (state=42000,code=4)
> 
> 
> Thanks,
> 
> Steve Moist
> 
>

Review Request 65641: SENTRY-853 Handle show grant on failure correctly

[Steve Moist via Review Board]

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

Review request for sentry.


Repository: sentry


Description
---

Improved the error message for when a show command is run instead of a generic 
message that is used improperly.


Diffs
-

  
sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/hadoop/hive/SentryHiveConstants.java
 38d1f468 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java
 1e520c0b 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java
 14a96191 
  
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java
 c75f57d6 


Diff: https://reviews.apache.org/r/65641/diff/1/


Testing
---

Setup cluster and ran commands through beeline:
0: jdbc:hive2://server> show grant user usercomedy;
Error: Error while compiling statement: FAILED: SemanticException Sentry does 
not allow privileges to be shown for: USER (state=42000,code=4)
0: jdbc:hive2://server> show grant on table movies;
Error: Error while compiling statement: FAILED: SemanticException Sentry does 
not allow privileges to be shown for: USER (state=42000,code=4)
0: jdbc:hive2://server> show grant on database moviesdb;
Error: Error while compiling statement: FAILED: SemanticException Sentry does 
not allow privileges to be shown for: USER (state=42000,code=4)
0: jdbc:hive2://server> show grant group comedy_group;
Error: Error while compiling statement: FAILED: SemanticException Sentry does 
not allow privileges to be shown for: GROUP (state=42000,code=4)
0: jdbc:hive2://server> show role grant role comedyrole;
Error: Error while compiling statement: FAILED: SemanticException Sentry does 
not allow privileges to be shown for: ROLE (state=42000,code=4)


Thanks,

Steve Moist

Review Request 65639: SENTRY-1242 Enable getting all privileges on a hive object

[Steve Moist via Review Board]

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

Review request for sentry.


Repository: sentry


Description
---

Enabled use of show grant on table and show grant on database for the current 
user.


Diffs
-

  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java
 1e520c0b 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java
 14a96191 
  
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryHiveAuthorizationTaskFactory.java
 c75f57d6 


Diff: https://reviews.apache.org/r/65639/diff/1/


Testing
---

Added unit tests, mvn clean package on sentry-binding/sentry-binding-hive


Thanks,

Steve Moist

Re: Review Request 65268: SENTRY-1904: TransactionManager should limit the max time spent by transaction retry

[Steve Moist via Review Board]

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




sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java
Lines 259 (patched)


If you're braking out of this anyway, why not just throw an Exception here 
instead of the break?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
Lines 102 (patched)


Change to 30_000 for readability or 30*1000



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
Lines 108 (patched)


change to 300_000 or 5*60*1000


- Steve Moist


On Jan. 23, 2018, 11:40 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65268/
> ---
> 
> (Updated Jan. 23, 2018, 11:40 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Na Li, Sergio Pena, and Vadim 
> Spector.
> 
> 
> Bugs: SENTRY-1904
> https://issues.apache.org/jira/browse/SENTRY-1904
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> The TransactionManager uses exponential backoff strategy for transaction 
> retries. This may cause some transactions to be delayed by a very long time. 
> We should also have a constraint on the max time for a transaction so that we 
> do not retry for too long. 
> 
> New patch that is attached adds upper bounds on below
> 
> 1.Interval between the retry attempts which increases exponentially.
> 2.Total time a transaction could spend in retries.
>  
> 
> With out these limits we would not have a control on how long a transaction 
> could be be active.
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
>  2f2b98412e7dfdcc847ffe7975a70f452554e747 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  edea5b64d8f98c93aafc1fe43fa97e00c2ce2948 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java
>  f4ff962a67f8a5c23cc5c8daa7bcb861d2e6b6a5 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
>  7e02874b4be6a7109108809b1f404fe971b7b8e2 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  b4100278392986c161625a366212c6fef66ec0a9 
> 
> 
> Diff: https://reviews.apache.org/r/65268/diff/3/
> 
> 
> Testing
> ---
> 
> Made sure all the tests pass.
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Re: Review Request 65388: SENTRY-2124 LeaderStatusMonitor.toString() throws IllegalFormatConversionException with AtomicLong

[Steve Moist via Review Board]

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


Ship it!




Ship It!

- Steve Moist


On Jan. 29, 2018, 4:51 p.m., Xinran Tinney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65388/
> ---
> 
> (Updated Jan. 29, 2018, 4:51 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Arjun Mishra, Colm O 
> hEigeartaigh, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> When I start Sentry, I noticed an exception on the console due to an illegal 
> format string in LeaderStatusMonitor.toString()
> java.util.IllegalFormatConversionException: d != 
> java.util.concurrent.atomic.AtomicLong 
> at java.util.Formatter$FormatSpecifier.failConversion(Formatter.java:4302) 
> at java.util.Formatter$FormatSpecifier.printInteger(Formatter.java:2793) 
> at java.util.Formatter$FormatSpecifier.print(Formatter.java:2747) 
> at java.util.Formatter.format(Formatter.java:2520) 
> at java.util.Formatter.format(Formatter.java:2455) 
> at java.lang.String.format(String.java:2940) 
> at 
> org.apache.sentry.provider.db.service.persistent.LeaderStatusMonitor.toString(LeaderStatusMonitor.java:282)
>  
> at 
> org.slf4j.helpers.MessageFormatter.safeObjectAppend(MessageFormatter.java:297)
>  
> at 
> org.slf4j.helpers.MessageFormatter.deeplyAppendParameter(MessageFormatter.java:269)
>  
> at org.slf4j.helpers.MessageFormatter.arrayFormat(MessageFormatter.java:227) 
> at org.slf4j.helpers.MessageFormatter.format(MessageFormatter.java:124) 
> at org.slf4j.impl.Log4jLoggerAdapter.info(Log4jLoggerAdapter.java:322) 
> at 
> org.apache.sentry.provider.db.service.persistent.LeaderStatusMonitor.takeLeadership(LeaderStatusMonitor.java:251)
>  
> at 
> sentry.org.apache.curator.framework.recipes.leader.LeaderSelector$WrappedListener.takeLeadership(LeaderSelector.java:537)
>  
> at 
> sentry.org.apache.curator.framework.recipes.leader.LeaderSelector.doWork(LeaderSelector.java:399)
>  
> at 
> sentry.org.apache.curator.framework.recipes.leader.LeaderSelector.doWorkLoop(LeaderSelector.java:444)
>  
> at 
> sentry.org.apache.curator.framework.recipes.leader.LeaderSelector.access$100(LeaderSelector.java:64)
>  
> at 
> sentry.org.apache.curator.framework.recipes.leader.LeaderSelector$2.call(LeaderSelector.java:245)
>  
> at 
> sentry.org.apache.curator.framework.recipes.leader.LeaderSelector$2.call(LeaderSelector.java:239)
>  
> at java.util.concurrent.FutureTask.run(FutureTask.java:266) 
> at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511) 
> at java.util.concurrent.FutureTask.run(FutureTask.java:266) 
> at 
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
>  
> at 
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
> at java.lang.Thread.run(Thread.java:748)
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/LeaderStatusMonitor.java
>  25a70bda 
> 
> 
> Diff: https://reviews.apache.org/r/65388/diff/1/
> 
> 
> Testing
> ---
> 
> mvn clean install
> 
> 
> Thanks,
> 
> Xinran Tinney
> 
>

Re: Review Request 65046: SENTRY-1819 HMSFollower and friends do not belong in sentry.service.thrift

[Steve Moist via Review Board]

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




sentry-dist/src/license/THIRD-PARTY.properties
Line 1 (original), 1 (patched)


Remove this file.  This should not be checked in.


- Steve Moist


On Jan. 9, 2018, 3:44 p.m., Xinran Tinney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65046/
> ---
> 
> (Updated Jan. 9, 2018, 3:44 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Arjun Mishra, Colm O 
> hEigeartaigh, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> We have several important classes - e.g. HMSFollower, NotificationProcessor, 
> CounterWait, LeaderStatusMonitor in the sentry.service.thrift package which 
> is weird - they should be in provider.db.service.persistent instead.
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
>  7565a34 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java
>  447deaf 
>   
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java
>  3bbf6fb 
>   
> sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SentrySolrPluginImpl.java
>  4092fe4 
>   
> sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java
>  5c2a301 
>   
> sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/binding/solr/TestSolrAuthzBinding.java
>  8d28ccc 
>   
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryGroupNotFoundException.java
>  6344435 
>   sentry-dist/src/license/THIRD-PARTY.properties 22429fc 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
>  eee4489 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java
>  73fcda8 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/GroupMappingService.java
>  9048d76 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/HadoopGroupMappingService.java
>  00b5cf6 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java
>  a9b98f3 
>   
> sentry-provider/sentry-provider-common/src/test/java/org/apache/sentry/provider/common/TestNoAuthorizationProvider.java
>  c8f2bed 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/CounterWait.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/LeaderStatusMonitor.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  6c4631f 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
>  2fbad36 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/CounterWait.java
>  558e695 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSClient.java
>  86ff47e 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
>  aa1b6a3 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveNotificationFetcher.java
>  097aa62 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveSimpleConnectionFactory.java
>  77634cf 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatusMonitor.java
>  8e80d55 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/NotificationProcessor.java
>  d09da5f 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
>  7e774b4 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  43535a7 
>   
> 

Re: Review Request 64259: SENTRY-1572 SentryMain() shouldn't dynamically load tool class

[Steve Moist via Review Board]

> On Jan. 4, 2018, 9:37 p.m., Arjun Mishra wrote:
> > I don't think you meant to commit 
> > "sentry-dist/src/license/THIRD-PARTY.properties". Am I right? It is being 
> > generated because of another issue. I am not sure if you intentionally 
> > created it

I have also seen this on another review as well, I also assume it's by 
accident, we should do something to prevent this unless there are actual 
changes to the liceneses.


- Steve


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


On Dec. 20, 2017, 4:54 p.m., Xinran Tinney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64259/
> ---
> 
> (Updated Dec. 20, 2017, 4:54 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Arjun Mishra, kalyan kumar 
> kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> TheSentryMain class currently works by mapping the command name to a Java 
> class that is then dynamically loaded:
> String commandName = commandLine.getOptionValue(COMMAND);
> String commandClazz = COMMANDS.get(commandName);
> Object command;
> try {
>   command = Class.forName(commandClazz.trim()).newInstance();
> } catch (Exception e) {
>   String msg = "Could not create instance of " + commandClazz + " for 
> command " + commandName;
>   throw new IllegalStateException(msg, e);
> }
> if (!(command instanceof Command)) {
>   String msg = "Command " + command.getClass().getName() + " is not an 
> instance of "
>   + Command.class.getName();
>   throw new IllegalStateException(msg);
> }
> ((Command)command).run(commandLine.getArgs());
>   }
> This ia too complicated and causes subtle problems at runtime. Instead it 
> should just create a new instance of appropriate class and call it directly.
> 
> 
> Diffs
> -
> 
>   bin/config_tool 4da85673 
>   bin/run_sentry.sh d58d5e5c 
>   bin/sentry 54e545aa 
>   
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryMain.java
>  3a981b2a 
>   sentry-dist/src/license/THIRD-PARTY.properties 22429fc3 
>   sentry-tools/pom.xml 45cfdb56 
> 
> 
> Diff: https://reviews.apache.org/r/64259/diff/3/
> 
> 
> Testing
> ---
> 
> mvn clean install, on cloudcat and all SUCCESS
> 
> 
> Thanks,
> 
> Xinran Tinney
> 
>

Re: Review Request 64949: SENTRY-641 Add binding for lily hbase indexer

[Steve Moist via Review Board]

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




sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
Lines 92 (patched)


What if they are empty strings?



sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
Lines 134 (patched)


Change to String.format for readability and not having to concat everything.



sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationComponent.java
Lines 23 (patched)


change to HBASE_INDEXER


- Steve Moist


On Jan. 4, 2018, 1:51 p.m., Mano Kovacs wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64949/
> ---
> 
> (Updated Jan. 4, 2018, 1:51 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-641
> https://issues.apache.org/jira/browse/SENTRY-641
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Binding for Lily Hbase Indexer, based on the implementation Gregory Chanan 
> created for CDH version of Sentry. The patch contains implementation that has 
> no dependency to Lily.
> 
> 
> Diffs
> -
> 
>   pom.xml c797181c8f123bc914ece27c13b70540978091bc 
>   sentry-binding/pom.xml b12683c1c742c989ac118e968c71df50366a8557 
>   sentry-binding/sentry-binding-hbase-indexer/pom.xml PRE-CREATION 
>   
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
>  PRE-CREATION 
>   
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/SentryHBaseIndexerAuthorizationException.java
>  PRE-CREATION 
>   
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/conf/HBaseIndexerAuthzConf.java
>  PRE-CREATION 
>   
> sentry-binding/sentry-binding-hbase-indexer/src/test/java/org/apache/sentry/binding/hbaseindexer/TestHBaseIndexerAuthzBinding.java
>  PRE-CREATION 
>   
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/log4j.properties
>  PRE-CREATION 
>   
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site-service.xml
>  PRE-CREATION 
>   
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site.xml
>  PRE-CREATION 
>   
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/test-authz-provider.ini
>  PRE-CREATION 
>   sentry-dist/pom.xml 63eb81c87172dc3237039b6d7641881b2831079f 
>   sentry-dist/src/license/THIRD-PARTY.properties 
> 22429fc3d644ea961d59260b4a08eb380d7a4325 
>   
> sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/IndexerWildcardPrivilege.java
>  9e9a0d2b3afc22098980b5a54b6a9989ad035b6f 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationComponent.java
>  5dc2b5592bdfbb281b083bdc835d455d07df0d86 
> 
> 
> Diff: https://reviews.apache.org/r/64949/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Mano Kovacs
> 
>

Re: Review Request 64317: SENTRY-2085: Keep sentry exceptions contained within Sentry

[Steve Moist via Review Board]

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




sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
Line 394 (original), 394 (patched)


Rename to getGroupsBySubject



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryGroupNotFoundException.java
Line 36 (original), 28 (patched)


Why rename it?



sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java
Line 72 (original), 73 (patched)


update javadoc for throws



sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/GroupMappingService.java
Line 34 (original), 35 (patched)


rename getGroupsForUser



sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java
Line 94 (original), 96 (patched)


replace with boolean hasAccess = doHasAccess(subject, 
authorizableHierarchy, actions, roleSet);

no need to initialize then set the value.


- Steve Moist


On Dec. 29, 2017, 11:30 p.m., Zachary Amsden wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64317/
> ---
> 
> (Updated Dec. 29, 2017, 11:30 p.m.)
> 
> 
> Review request for sentry and Na Li.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Instead of leaking new exceptions outside the API, use the
> existing authorization exceptions to indicate authorization
> failure when a user has no group configured.
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
>  8ce7a02 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java
>  9c60c22 
>   
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java
>  a41d1bd 
>   
> sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SentrySolrPluginImpl.java
>  91d08f0 
>   
> sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java
>  803e5ea 
>   
> sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/binding/solr/TestSolrAuthzBinding.java
>  f060b82 
>   
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryGroupNotFoundException.java
>  b978df6 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java
>  2d82bcf 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/GroupMappingService.java
>  7e85261 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/HadoopGroupMappingService.java
>  bde53d5 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java
>  005724f 
>   
> sentry-provider/sentry-provider-common/src/test/java/org/apache/sentry/provider/common/TestNoAuthorizationProvider.java
>  fe01b06 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
>  650880b 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java
>  6597a7c 
>   
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/LocalGroupMappingService.java
>  5447420 
>   
> sentry-provider/sentry-provider-file/src/test/java/org/apache/sentry/provider/file/TestLocalGroupMapping.java
>  9864b82 
>   
> sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/component/QueryDocAuthorizationComponent.java
>  2338ab8 
>   
> sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java
>  02ac514 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java
>  02ac514 
> 
> 
> Diff: https://reviews.apache.org/r/64317/diff/4/
> 
> 
> Testing
> ---
> 
> Running JUnit tests with mvn install.
> 
> 
> Thanks,
> 
> Zachary Amsden
> 
>

Re: Review Request 64890: SENTRY-1165 add clover plugin to maven to get code coverage report

[Steve Moist via Review Board]

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




pom.xml
Lines 117 (patched)


Looks like 4.2.1 is the latest version, any reason why we're not using it?


- Steve Moist


On Jan. 2, 2018, 4:43 p.m., Xinran Tinney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64890/
> ---
> 
> (Updated Jan. 2, 2018, 4:43 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> add clover plugin to maven to get code coverage report
> 
> 
> Diffs
> -
> 
>   pom.xml c797181c 
> 
> 
> Diff: https://reviews.apache.org/r/64890/diff/1/
> 
> 
> Testing
> ---
> 
> mvn clean install on testing cluster and all the tests past.
> The steps for generating a clover report is as folllows: (Since clover has 
> become an opensource now, the one used is openclover)
> 1. Put
> 
> org.openclover
> 
> In maven/conf/settings.xml
> 2. It is required to first create a Clover database, run 
> mvn clover:instrument
> 3. To generate report, run 
> mvn clover:clover
> The report is in each module's folder e.g., 
> .../sentry/sentry-provider/sentry-provider-db/target/site/clover/clover.xml'
> 
> 
> Thanks,
> 
> Xinran Tinney
> 
>

Re: Review Request 64452: SENTRY-2091: User-based Privilege is broken by SENTRY-769

[Steve Moist via Review Board]

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




sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 1896 (original), 1896 (patched)


Can replace with CollectionUtils.isNotEmpty



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 1912 (original), 1912 (patched)


Can replace with CollectionUtils.isNotEmpty(users)



sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestGrantUserToRole.java
Lines 280 (patched)


Worth it to add @Test(expected=HiveSQLException) or verify exception 
message?


- Steve Moist


On Dec. 14, 2017, 8:54 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64452/
> ---
> 
> (Updated Dec. 14, 2017, 8:54 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, Sergio 
> Pena, and Vadim Spector.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Update the code to make sure user without group but with role has proper 
> access. 
> test case is added for the scenario that user has no group but with desired 
> role. 
> test case is added for the scenario that user has no group and no privilege 
> to allow the access
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingHookBaseV2.java
>  5a21dd3 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java
>  9c60c22 
>   
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java
>  a41d1bd 
>   
> sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/binding/solr/TestSolrAuthzBinding.java
>  f060b82 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java
>  005724f 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  6c4631f 
>   
> sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java
>  02ac514 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestGrantUserToRole.java
>  5364937 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java
>  02ac514 
> 
> 
> Diff: https://reviews.apache.org/r/64452/diff/2/
> 
> 
> Testing
> ---
> 
> unit tests
> 
> 
> Thanks,
> 
> Na Li
> 
>

Re: Review Request 64661: SENTRY-1944: Optimize DelegateSentryStore.getGroupsByRoles()

[Steve Moist via Review Board]

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




sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
Lines 263 (patched)


Any reason why this is needed when this method returns a list of strings 
and not the actual objects?


- Steve Moist


On Dec. 19, 2017, 8:04 p.m., Arjun Mishra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64661/
> ---
> 
> (Updated Dec. 19, 2017, 8:04 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Brian Towles, kalyan kumar 
> kalvagadda, Na Li, Sergio Pena, and Vadim Spector.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> When Solr is using Sentry server for authorization, it issues a lot of calls 
> to {{getGroupsByRoles()}} function in {{DelegateSentryStore}}.
> 
> This function isn't very efficient - it walks over each role in the set, 
> obtains role by name, get groups for each role, and collects all group names 
> into a set.
> 
> It may be possible to optimize it.
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
>  4cb46abc5 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestDelegateSentryStore.java
>  69d16238f 
> 
> 
> Diff: https://reviews.apache.org/r/64661/diff/4/
> 
> 
> Testing
> ---
> 
> mvn -f sentry-provider/sentry-provider-db/pom.xml test
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 63775: SENTRY-1543 dropOrRenamePrivilegeForAllRoles() has confusing code

[Steve Moist via Review Board]

> On Nov. 15, 2017, 7:11 p.m., Sergio Pena wrote:
> > The subject of the review board must match the JIRA subject. You could 
> > either change the JIRA or the Review subject to match the correct fix.
> > 
> > The getUniqueMSentryPrivilege() is still confusing. Any idea of the 
> > difference in both methods? Seems getMSentryPrivileges() lacks of the grant 
> > option and unique query. The unique query is used to avoid datanucleus to 
> > return a list, so this is ok, but how does the grant option being the only 
> > difference between a list and a single privilege?
> 
> Steve Moist wrote:
> My understanding is that getMSentryPrivileges is mainly used to get all 
> the associated roles for a privelege.  I'll take a deeper look into how grant 
> affects it.

Suppose theres a database(db1) with 2 tables(tbl1, tbl2) that have columns. 
When a user renames a column(db1.tbl1.col1) in Hive the privileges for 
db1.tbl1.col1 need to be updated, the query to do so needs to be pretty 
specific to find the right column.  So if they were to simply search for  col1, 
it would affect also db1.tbl2.col1.  So it needs to query for db, table, and 
column.  With that said, it also needs to be able to rename all of the columns 
with different actions, grants, etc.  In the case of a database rename, it 
needs to get all where db1 = oldname.  So it needs to be as precise and wide as 
it can be for this to happen.


- Steve


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


On Nov. 15, 2017, 9:47 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63775/
> ---
> 
> (Updated Nov. 15, 2017, 9:47 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-1543
> https://issues.apache.org/jira/browse/SENTRY-1543
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Renamed methods to be more clear in their use since they both have the same 
> name but function and query differently.
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  7217dea7 
> 
> 
> Diff: https://reviews.apache.org/r/63775/diff/1/
> 
> 
> Testing
> ---
> 
> Unit tests ran.
> 
> 
> Thanks,
> 
> Steve Moist
> 
>

Re: Review Request 64259: SENTRY-1572 SentryMain() shouldn't dynamically load tool class

[Steve Moist via Review Board]

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



This project has a lot of command line tools, maybe rename to 
sentry-command-line-tools or something of that effect?


pom.xml
Lines 647 (patched)


whitespace



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryMain.java
Lines 128 (patched)


Command won't be null as printHelp does a system.exit.


- Steve Moist


On Dec. 1, 2017, 9:15 p.m., Xinran Tinney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64259/
> ---
> 
> (Updated Dec. 1, 2017, 9:15 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Arjun Mishra, kalyan kumar 
> kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> TheSentryMain class currently works by mapping the command name to a Java 
> class that is then dynamically loaded:
> String commandName = commandLine.getOptionValue(COMMAND);
> String commandClazz = COMMANDS.get(commandName);
> Object command;
> try {
>   command = Class.forName(commandClazz.trim()).newInstance();
> } catch (Exception e) {
>   String msg = "Could not create instance of " + commandClazz + " for 
> command " + commandName;
>   throw new IllegalStateException(msg, e);
> }
> if (!(command instanceof Command)) {
>   String msg = "Command " + command.getClass().getName() + " is not an 
> instance of "
>   + Command.class.getName();
>   throw new IllegalStateException(msg);
> }
> ((Command)command).run(commandLine.getArgs());
>   }
> This ia too complicated and causes subtle problems at runtime. Instead it 
> should just create a new instance of appropriate class and call it directly.
> 
> 
> Diffs
> -
> 
>   bin/config_tool 4da85673 
>   bin/run_sentry.sh d58d5e5c 
>   bin/sentry 54e545aa 
>   pom.xml dd408d85 
>   
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryMain.java
>  3a981b2a 
>   sentry-main/pom.xml PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/64259/diff/1/
> 
> 
> Testing
> ---
> 
> mvn clean install, on cloudcat and all SUCCESS
> 
> 
> Thanks,
> 
> Xinran Tinney
> 
>

Re: Review Request 63775: SENTRY-1543 dropOrRenamePrivilegeForAllRoles() has confusing code

[Steve Moist via Review Board]

> On Nov. 15, 2017, 7:11 p.m., Sergio Pena wrote:
> > The subject of the review board must match the JIRA subject. You could 
> > either change the JIRA or the Review subject to match the correct fix.
> > 
> > The getUniqueMSentryPrivilege() is still confusing. Any idea of the 
> > difference in both methods? Seems getMSentryPrivileges() lacks of the grant 
> > option and unique query. The unique query is used to avoid datanucleus to 
> > return a list, so this is ok, but how does the grant option being the only 
> > difference between a list and a single privilege?

My understanding is that getMSentryPrivileges is mainly used to get all the 
associated roles for a privelege.  I'll take a deeper look into how grant 
affects it.


- Steve


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


On Nov. 15, 2017, 9:47 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63775/
> ---
> 
> (Updated Nov. 15, 2017, 9:47 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: SENTRY-1543
> https://issues.apache.org/jira/browse/SENTRY-1543
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Renamed methods to be more clear in their use since they both have the same 
> name but function and query differently.
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  7217dea7 
> 
> 
> Diff: https://reviews.apache.org/r/63775/diff/1/
> 
> 
> Testing
> ---
> 
> Unit tests ran.
> 
> 
> Thanks,
> 
> Steve Moist
> 
>

Re: Review Request 63964: SENTRY-2049: Remove hive-authz2 profile from the sentry-dist module

[Steve Moist via Review Board]

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

(Updated Nov. 27, 2017, 9:36 p.m.)


Review request for sentry.


Bugs: SENTRY-2049
https://issues.apache.org/jira/browse/SENTRY-2049


Repository: sentry


Description
---

Remove hive-authz2 profile from the sentry-dist module


Diffs (updated)
-

  sentry-dist/pom.xml f4662957 


Diff: https://reviews.apache.org/r/63964/diff/2/

Changes: https://reviews.apache.org/r/63964/diff/1-2/


Testing
---

unit tests ran.


Thanks,

Steve Moist

Review Request 63964: SENTRY-2049: Remove hive-authz2 profile from the sentry-dist module

[Steve Moist via Review Board]

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

Review request for sentry.


Bugs: SENTRY-2049
https://issues.apache.org/jira/browse/SENTRY-2049


Repository: sentry


Description
---

Remove hive-authz2 profile from the sentry-dist module


Diffs
-

  sentry-dist/pom.xml 69f4fcca 


Diff: https://reviews.apache.org/r/63964/diff/1/


Testing
---

unit tests ran.


Thanks,

Steve Moist

Re: Review Request 63775: SENTRY-1543 dropOrRenamePrivilegeForAllRoles() has confusing code

[Steve Moist via Review Board]

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

(Updated Nov. 15, 2017, 9:47 p.m.)


Review request for sentry.


Summary (updated)
-

SENTRY-1543 dropOrRenamePrivilegeForAllRoles() has confusing code


Bugs: SENTRY-1543
https://issues.apache.org/jira/browse/SENTRY-1543


Repository: sentry


Description
---

Renamed methods to be more clear in their use since they both have the same 
name but function and query differently.


Diffs
-

  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
 7217dea7 


Diff: https://reviews.apache.org/r/63775/diff/1/


Testing
---

Unit tests ran.


Thanks,

Steve Moist

Review Request 63775: SENTRY-1543 Rename methods to be clearer

[Steve Moist via Review Board]

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

Review request for sentry.


Bugs: SENTRY-1543
https://issues.apache.org/jira/browse/SENTRY-1543


Repository: sentry


Description
---

Renamed methods to be more clear in their use since they both have the same 
name but function and query differently.


Diffs
-

  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
 7217dea7 


Diff: https://reviews.apache.org/r/63775/diff/1/


Testing
---

Unit tests ran.


Thanks,

Steve Moist

Re: Review Request 63704: SENTRY-1662 Constants java uses mutable collection

[Steve Moist via Review Board]

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

(Updated Nov. 9, 2017, 7:32 p.m.)


Review request for sentry.


Bugs: SENTRY-1662
https://issues.apache.org/jira/browse/SENTRY-1662


Repository: sentry


Description
---

Changes the hash maps to be unmodifable maps.


Diffs (updated)
-

  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/Constants.java
 2e71ce02 


Diff: https://reviews.apache.org/r/63704/diff/2/

Changes: https://reviews.apache.org/r/63704/diff/1-2/


Testing
---

Unit tests ran.


Thanks,

Steve Moist

Review Request 63704: SENTRY-1662 Constants java uses mutable collection

[Steve Moist via Review Board]

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

Review request for sentry.


Bugs: SENTRY-1662
https://issues.apache.org/jira/browse/SENTRY-1662


Repository: sentry


Description
---

Changes the hash maps to be unmodifable maps.


Diffs
-

  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/Constants.java
 2e71ce02 


Diff: https://reviews.apache.org/r/63704/diff/1/


Testing
---

Unit tests ran.


Thanks,

Steve Moist