> On Jan. 19, 2018, 4:12 a.m., Colm O hEigeartaigh wrote:
> > The patch does not apply for me on master:
> > 
> > Also, the following version could be changed to the version 
> > ${jackson-fasterxml.version}:
> > 
> > <dependency>
> > +        <groupId>com.fasterxml.jackson.core</groupId>
> > +        <artifactId>${jackson-fasterxml.version}</artifactId>
> > +        <version>2.2.2</version>
> > +      </dependency>
> > +      <dependency>
> > 
> > 
> > In general, what is the motivation behind the patch? It seems to me it'll 
> > make things way more complicated to update a third party dependency, as we 
> > will have to analyse which of its dependencies have also changed and modify 
> > the Sentry poms accordingly.

Fyi this was moved to https://reviews.apache.org/r/65244/ because of master 
changes with paths to some files.


The one of the primary motivations for this patch is to help on its way to 
cleaning up the distribution.  Currently the dist module reads all dependencies 
no mater what scope they are and drops them into the distributed libs.  This 
causes things like junit and ant to be pushed into the libs that are being 
distributed.  With the changes to have direct dependencies always defined it 
allows us to take compile and runtime scopes only into account when dropping 
the libs needed.  


As well this identifies which libraries are provided already by environments 
where the plugins/bindings are going into.  For example in the hive bindings, 
the hive and hadoop libraries need only be defined with "provided" scope, since 
with those application we want to use the hadoop and hive libraries that the 
applications already provide.


This makes it a lot easier for shading and package shifting of the binding and 
plugins for libraries and versions of those libraries that are needed by the 
binding and might conflict with versions already in the application which the 
binding or plugin is going into.  Guava is a major issue with this.  Doing this 
short of shading based on the cleanup would allow us to rev Guava and use newer 
Guava features while not conflicting with the Guava version the application is 
using.  By having the directly used dependency defined it gives us control over 
the exact version we are using and not be dependent on and having conflicts 
with the transitive dependencies of the application being embedded in.


This patch will not really make the development process harder since the 
analysis of the dependencies needed automatically runs as part of the build and 
a failure occurs telling you which "used but undefined" and which "defined but 
unused" libraries are missing or in the pom.  There is even an xml dump of the 
dependencies part need to put right into the pom. No additional runs or 
dependency analysis needs to take place.


- Brian


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


On Jan. 17, 2018, 2:42 p.m., Brian Towles wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65192/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2018, 2:42 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Arjun Mishra, Colm O 
> hEigeartaigh, kalyan kumar kalvagadda, Na Li, Sergio Pena, and Vadim Spector.
> 
> 
> Bugs: SENTRY-2074
>     https://issues.apache.org/jira/browse/SENTRY-2074
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Created a patch that has all of the direct dependency libraries explicitly 
> defined.  There is also some minor cleanup on ThreadSafe annotations to make 
> it from a consistent package (this is only used by IDEs for checks anyways).  
> As well I added a check in the poms to fail a build if it discovers any 
> direct use dependencies that are not defined in the pom.
> 
> 
> Diffs
> -----
> 
>   pom.xml 6f9856e45b72ef9e0c43a222eddc8452b64f1a71 
>   sentry-binding/pom.xml 17b0f1afa658203327668b33990e5da53dd75ad0 
>   sentry-binding/sentry-binding-hbase-indexer/pom.xml 
> d50acfec954acf8e8d9682425122039e6f5a2724 
>   sentry-binding/sentry-binding-hive-common/pom.xml 
> e39004bd1681f1424a463b59d05359206b94777b 
>   sentry-binding/sentry-binding-hive-conf/pom.xml 
> 3e7e70a321733a97914f13f9cc5e39557ab1c9fb 
>   sentry-binding/sentry-binding-hive-follower/pom.xml 
> 5f8a5afb479b783d027a005ea8f72590445b8386 
>   sentry-binding/sentry-binding-hive/pom.xml 
> ccfa9cfeab3cafcccd275a5b395ca6eb3f7aa609 
>   sentry-binding/sentry-binding-kafka/pom.xml 
> 239eeba5fe28962e6481dff8f5b5bd1303e8666a 
>   sentry-binding/sentry-binding-solr/pom.xml 
> f08669994aa6d058fc9c92e6a8b576063602cc95 
>   sentry-binding/sentry-binding-sqoop/pom.xml 
> 82cd4a6fdb330ae2162e1c257f5e47638e1a3992 
>   sentry-core/sentry-core-common/pom.xml 
> 75ce574ae0e2296e4bb6c8d4ec1792d43969e00f 
>   sentry-core/sentry-core-model-db/pom.xml 
> 30519aa984f3d24b38e16641a38f9436e54594bc 
>   sentry-core/sentry-core-model-indexer/pom.xml 
> f65e49af71de8aa0b98f22cf64d57902265af1b3 
>   sentry-core/sentry-core-model-kafka/pom.xml 
> cfe9221400116acba2ad0e299b4774151dc5159a 
>   sentry-core/sentry-core-model-solr/pom.xml 
> 95ea02bed613cd81ff8b127f9b35f9887892369d 
>   sentry-core/sentry-core-model-sqoop/pom.xml 
> 5629028b94eb3aa11c17de40032c00c39ab2bb85 
>   sentry-dist/pom.xml 8cc4ecaf65f900e95601efab2331416c60e1cdae 
>   sentry-dist/src/main/assembly/bin.xml 
> 72773df1e26d9658bf9c1291574648ad8e551eac 
>   sentry-hdfs/pom.xml a015e11d37f688657751a47379ef6c77a22035e9 
>   sentry-hdfs/sentry-hdfs-common/pom.xml 
> 58719c399f026ebddef7b91b3abdeccf936c619a 
>   sentry-hdfs/sentry-hdfs-dist/pom.xml 
> 93943157d8f93d7049a23e5cbf90eb76258f6a90 
>   sentry-hdfs/sentry-hdfs-namenode-plugin/pom.xml 
> e1bd8c304fef2be1e165622b6cdc95250e75fa5d 
>   sentry-hdfs/sentry-hdfs-service/pom.xml 
> 50a451d917eb7428cd71ab7cedd9c4363111c166 
>   sentry-policy/pom.xml 112482900097daf8db7291134493f4acb8cd9c74 
>   sentry-policy/sentry-policy-common/pom.xml 
> 902f3e6f933295d64053e2a74e3191e9fa121133 
>   
> sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/PolicyEngine.java
>  504b5ea3f30328cd84adf99543dca59630286068 
>   sentry-policy/sentry-policy-engine/pom.xml 
> 5bdd10bc1116dd1d3551d5b10869296632fd903d 
>   sentry-provider/pom.xml b075f49b5d711e9b85cc20e0b8dacd1977b530b0 
>   sentry-provider/sentry-provider-cache/pom.xml 
> e278878955f9ab91df44094e7238e21c7cca7105 
>   sentry-provider/sentry-provider-common/pom.xml 
> f2698738bec0261621b975b6872ffae592b9adf5 
>   sentry-provider/sentry-provider-db/pom.xml 
> db4808ddcab8e6d1554f7ddd6c2de224d263fed6 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/CounterWait.java
>  558e69534cc1e73c5e918220459f3155555a403a 
>   sentry-provider/sentry-provider-file/pom.xml 
> 6623ffe74d48da5db64e6634619f5a4cf9463a30 
>   sentry-solr/pom.xml df8c39748510b797a5331322cdab99c0ce8e2c89 
>   sentry-solr/solr-sentry-handlers/pom.xml 
> accd581263447578724b5b7d7699f9e42efed083 
>   sentry-tests/pom.xml e17f2a8918be63fcf3373a124bcbef7ddd548c2e 
>   sentry-tests/sentry-tests-hive-v2/pom.xml 
> b129ed67dd2869317fdee33e18844f376f0fb1a5 
>   sentry-tests/sentry-tests-hive/pom.xml 
> 6816249da67a3fe9d0bc97217c5d8e8e184788a0 
>   sentry-tests/sentry-tests-kafka/pom.xml 
> e61f64db0c6787d09874235d061061add1b746d0 
>   
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/KafkaTestServer.java
>   
>   
> sentry-tests/sentry-tests-kafka/src/main/java/org/apache/sentry/tests/e2e/kafka/TestUtils.java
>   
>   sentry-tests/sentry-tests-solr/pom.xml 
> 5ef7a2b1de67a2f35510ad41c0150ad1bc957118 
>   sentry-tests/sentry-tests-sqoop/pom.xml 
> eed8269d4e1b67fd622d539741e50d9984ae90c8 
>   sentry-tools/pom.xml 45cfdb562f8cb9955d14165240ca0f7b33028551 
> 
> 
> Diff: https://reviews.apache.org/r/65192/diff/1/
> 
> 
> Testing
> -------
> 
> Full unit test passing and builds
> 
> 
> Thanks,
> 
> Brian Towles
> 
>

Reply via email to