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

2018-07-09 Thread kalyan kumar kalvagadda via Review Board 

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


Ship it!




Ship It!

- kalyan kumar kalvagadda


On July 9, 2018, 7:28 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: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
> ---
> 
> 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

2018-07-09 Thread 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

2018-07-09 Thread 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

2018-07-09 Thread 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

2018-07-09 Thread Alexander Kolbasov 

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




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?


- Alexander Kolbasov


On July 6, 2018, 7:06 p.m., Steve Moist wrote:
> 
> ---
> 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
> -
> 
>   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/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve Moist
> 
>

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

2018-07-06 Thread kalyan kumar kalvagadda via Review Board 

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


Fix it, then Ship it!




Fix it.


sentry-binding/sentry-binding-hive-follower/pom.xml
Lines 63-68 (patched)


It's good to move all the exclusions to parent pom.

Let's take this example. If some one other packe tries to include 
hive-catelog-server-extensions they again bring the dependecy on metrics-core 
which could be avoided if the exlusion were added in parent pom.



sentry-binding/sentry-binding-kafka/pom.xml
Line 70 (original), 70-76 (patched)


It's good to move all the exclusions to parent pom.


- kalyan kumar kalvagadda


On July 6, 2018, 7:06 p.m., Steve Moist wrote:
> 
> ---
> 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
> -
> 
>   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/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve Moist
> 
>

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

2018-07-06 Thread 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

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

2018-07-06 Thread kalyan kumar kalvagadda via Review Board 

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




sentry-binding/sentry-binding-hive/pom.xml
Lines 63-68 (patched)


Adding this exclusion in parent would avoid excluding in multiple child pom 
files. This is a general comment that applies to other changes in the patch.


- kalyan kumar kalvagadda


On July 6, 2018, 4:29 p.m., Steve Moist wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67846/
> ---
> 
> (Updated July 6, 2018, 4:29 p.m.)
> 
> 
> 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
> 
>