You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org> on 2018/11/01 13:39:29 UTC

Re: Review Request 69212: SENTRY-2329: Integrate sentry with Hadoop 3.1.1

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




pom.xml
Lines 216-231 (patched)
<https://reviews.apache.org/r/69212/#comment294887>

    I think hadoop-minicluster can be made a test only dependency as it is used only for testing by changing the scope to test.
    
    If we are good with this we don't have to exclude these as these dependencies are not propogated anyway.



sentry-binding/sentry-binding-solr/pom.xml
Lines 32-47 (patched)
<https://reviews.apache.org/r/69212/#comment294888>

    Can you elaborate on this?
    
    If the scope of hadoop-minicluster is changed to test, would there stil be issue with the jetty version?



sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryINodeAttributesProvider.java
Lines 334-338 (patched)
<https://reviews.apache.org/r/69212/#comment294884>

    What made you add this check?
    
    Was there something that was failing OR Just added for safety?


- kalyan kumar kalvagadda


On Oct. 30, 2018, 2:16 a.m., Sergio Pena wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69212/
> -----------------------------------------------------------
> 
> (Updated Oct. 30, 2018, 2:16 a.m.)
> 
> 
> Review request for sentry and kalyan kumar kalvagadda.
> 
> 
> Bugs: sentry-2329
>     https://issues.apache.org/jira/browse/sentry-2329
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Bump hadoop.version to 3.1.1. The changes on Sentry are compatible with Hadoop 2.7.
> 
> 
> Diffs
> -----
> 
>   pom.xml acbdcc2722bf189811cb528ac1b2d07983a571c2 
>   sentry-binding/sentry-binding-solr/pom.xml f2a5fca76d3d220fcf2b72a3179ff5218fc6577c 
>   sentry-hdfs/sentry-hdfs-common/pom.xml df6f04c048b502ff5f8e8ec397d75166faba8c3c 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java c9ecc4021b167d98c7dade409c97ae7d26e967ea 
>   sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryINodeAttributesProvider.java 18b62652a11dfee6683cb8f24944ccd3d344dc9f 
>   sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryWebServerWithKerberos.java 5d94d4bc6a2a47189e69556a5e4d9bdee05952a7 
> 
> 
> Diff: https://reviews.apache.org/r/69212/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>


Re: Review Request 69212: SENTRY-2329: Integrate sentry with Hadoop 3.1.1

Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.

> On Nov. 1, 2018, 1:39 p.m., kalyan kumar kalvagadda wrote:
> > pom.xml
> > Lines 216-231 (patched)
> > <https://reviews.apache.org/r/69212/diff/1/?file=2103118#file2103118line217>
> >
> >     I think hadoop-minicluster can be made a test only dependency as it is used only for testing by changing the scope to test.
> >     
> >     If we are good with this we don't have to exclude these as these dependencies are not propogated anyway.

Yeah, that made the trick easier. I will update it to be a scope for test instead.


> On Nov. 1, 2018, 1:39 p.m., kalyan kumar kalvagadda wrote:
> > sentry-binding/sentry-binding-solr/pom.xml
> > Lines 32-47 (patched)
> > <https://reviews.apache.org/r/69212/diff/1/?file=2103119#file2103119line32>
> >
> >     Can you elaborate on this?
> >     
> >     If the scope of hadoop-minicluster is changed to test, would there stil be issue with the jetty version?

I did not work. I couldn't figure out what jetty dependencies were conflicting. Keepting these at the beginning solves the issue.


> On Nov. 1, 2018, 1:39 p.m., kalyan kumar kalvagadda wrote:
> > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryINodeAttributesProvider.java
> > Lines 334-338 (patched)
> > <https://reviews.apache.org/r/69212/diff/1/?file=2103122#file2103122line334>
> >
> >     What made you add this check?
> >     
> >     Was there something that was failing OR Just added for safety?

Hadoop3 sends an empty pathElements array so without this check the method fails. It is safe to add it, though.


- Sergio


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


On Oct. 30, 2018, 2:16 a.m., Sergio Pena wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69212/
> -----------------------------------------------------------
> 
> (Updated Oct. 30, 2018, 2:16 a.m.)
> 
> 
> Review request for sentry and kalyan kumar kalvagadda.
> 
> 
> Bugs: sentry-2329
>     https://issues.apache.org/jira/browse/sentry-2329
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Bump hadoop.version to 3.1.1. The changes on Sentry are compatible with Hadoop 2.7.
> 
> 
> Diffs
> -----
> 
>   pom.xml acbdcc2722bf189811cb528ac1b2d07983a571c2 
>   sentry-binding/sentry-binding-solr/pom.xml f2a5fca76d3d220fcf2b72a3179ff5218fc6577c 
>   sentry-hdfs/sentry-hdfs-common/pom.xml df6f04c048b502ff5f8e8ec397d75166faba8c3c 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java c9ecc4021b167d98c7dade409c97ae7d26e967ea 
>   sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryINodeAttributesProvider.java 18b62652a11dfee6683cb8f24944ccd3d344dc9f 
>   sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryWebServerWithKerberos.java 5d94d4bc6a2a47189e69556a5e4d9bdee05952a7 
> 
> 
> Diff: https://reviews.apache.org/r/69212/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>