You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Sergio Pena via Review Board <no...@reviews.apache.org> on 2018/10/30 02:16:22 UTC
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/
-----------------------------------------------------------
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
>
>
Re: Review Request 69212: SENTRY-2329: Integrate sentry with Hadoop
3.1.1
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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, 9:26 p.m., Na Li wrote:
> > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java
> > Line 143 (original)
> > <https://reviews.apache.org/r/69212/diff/1/?file=2103121#file2103121line145>
> >
> > why do you remove it? Is it impossible to throw this exception?
It's not needed. The change is not using URIUtil anymore, so there is no URI exception.
- Sergio
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69212/#review210262
-----------------------------------------------------------
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 Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69212/#review210262
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java
Line 143 (original)
<https://reviews.apache.org/r/69212/#comment294917>
why do you remove it? Is it impossible to throw this exception?
- Na Li
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 kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69212/#review210331
-----------------------------------------------------------
Sergio,
I remeber we taking about supporting multiple versions of Hadoop somehow but this change is not backward compatible. People using sentry 2.2 are forced to use Hadoop 3.1.1 only. Is that correct?
- kalyan kumar kalvagadda
On Nov. 2, 2018, 6:40 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69212/
> -----------------------------------------------------------
>
> (Updated Nov. 2, 2018, 6:40 p.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/2/
>
>
> 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>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69212/
-----------------------------------------------------------
(Updated Nov. 5, 2018, 8 p.m.)
Review request for sentry and kalyan kumar kalvagadda.
Bugs: sentry-2329
https://issues.apache.org/jira/browse/sentry-2329
Repository: sentry
Description (updated)
-------
Bump hadoop.version to 3.1.1.
This integration is compatible with old Hadoop 2.x versions. You can compile with Hadoop 2.x by
changing the pom.xml or you can add the Sentry/HDFS binding jars built with Hadoop3 in the
Hadoop 2 classpath. Both ways are verified and working.
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/2/
Testing
-------
Thanks,
Sergio Pena
Re: Review Request 69212: SENTRY-2329: Integrate sentry with Hadoop
3.1.1
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69212/#review210333
-----------------------------------------------------------
Ship it!
Ship It!
- kalyan kumar kalvagadda
On Nov. 2, 2018, 6:40 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69212/
> -----------------------------------------------------------
>
> (Updated Nov. 2, 2018, 6:40 p.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/2/
>
>
> 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>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69212/
-----------------------------------------------------------
(Updated Nov. 2, 2018, 6:40 p.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 (updated)
-----
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/2/
Changes: https://reviews.apache.org/r/69212/diff/1-2/
Testing
-------
Thanks,
Sergio Pena