----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65244/#review196040 -----------------------------------------------------------
Thanks for the explanation - I see the dependency list is greatly reduced in the distribution, nice! A few comments/questions: a) Please remove the whitespace changes: warning: squelched 48 whitespace errors warning: 53 lines add whitespace errors. b) commons-httpclinet.version in the root pom should be "commons-httpclient.version" c) Are the folllowing jars really required? zookeeper-3.4.6-tests.jar curator-test-2.11.1.jar d) Do we need both Stax API jars? stax-api-1.0.1.jar stax-api-1.0-2.jar e) Can we use a consistent Jackson version? jackson-annotations-2.2.2.jar jackson-core-2.2.2.jar jackson-core-asl-1.8.8.jar jackson-databind-2.2.2.jar jackson-jaxrs-1.8.3.jar jackson-mapper-asl-1.8.8.jar jackson-xc-1.8.3.jar - Colm O hEigeartaigh On Jan. 22, 2018, 3:12 p.m., Brian Towles wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65244/ > ----------------------------------------------------------- > > (Updated Jan. 22, 2018, 3:12 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 > ------- > > Resubmitted from https://reviews.apache.org/r/65192/ in order to fix changed > paths file from master. > > 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. > > 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. > > > 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/license/THIRD-PARTY.properties > 7226acdf90f8c38c33ed9da07dd5db805edb0096 > sentry-dist/src/main/assembly/bin.xml > 72773df1e26d9658bf9c1291574648ad8e551eac > sentry-hdfs/pom.xml a015e11d37f688657751a47379ef6c77a22035e9 > sentry-hdfs/sentry-hdfs-common/pom.xml > 5c6c96c46ee384ba5981611333741fde3bd59e10 > 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 > 5733445af481fd83bb71189178647af234fe77a1 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/CounterWait.java > d8c82970b56b1599a07f0e26edab8ed3d59b9948 > 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/65244/diff/1/ > > > Testing > ------- > > Full unit test passing and builds > > > Thanks, > > Brian Towles > >
