Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/10748 )
Change subject: IMPALA-7180: Pin Impala CDH dependencies ...................................................................... Patch Set 7: Code-Review+1 (7 comments) > Patch Set 5: Code-Review+1 > > (7 comments) > > I think this is fine. I have some nits, but nothing earth-shattering. > > When the CDH_BUILD_NUMBER variable changes upstream, will people need to > re-build their miniclusters? I don't know if we're going to like have the > build numbers in impala-config.sh. On one hand, it's reproducible, but on the > other, it may have annoying noisy commits and so on. They may need to run a build but reformatting the test data is not necessary (although recommended to make sure the clients and servers are in coherent) unless there is something major in the CDH components that require re-creating the test data. It should work similarly to the way it works right now. Carrying Phil's +1 http://gerrit.cloudera.org:8080/#/c/10748/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10748/5//COMMIT_MSG@15 PS5, Line 15: CDH dependencies by storing only the CDH tarballs in S3. The Maven > nit: lowercase pin Done http://gerrit.cloudera.org:8080/#/c/10748/5//COMMIT_MSG@16 PS5, Line 16: repository will still use https://repository.cloudera.com, so there > nit:", so" across this and the next line. Done http://gerrit.cloudera.org:8080/#/c/10748/5//COMMIT_MSG@28 PS5, Line 28: This patch also fixes dependency issues in Hadoop that transitively > Let's be explicit about json-smart in here. Also reminds me of IMPALA-7120. Done http://gerrit.cloudera.org:8080/#/c/10748/5/bin/bootstrap_toolchain.py File bin/bootstrap_toolchain.py: http://gerrit.cloudera.org:8080/#/c/10748/5/bin/bootstrap_toolchain.py@403 PS5, Line 403: - llama-minikdc (downloaded from $TOOLCHAIN_HOST) > nit: move line 403 into line 397 "following CDH components into the directo Done http://gerrit.cloudera.org:8080/#/c/10748/5/impala-parent/pom.xml File impala-parent/pom.xml: http://gerrit.cloudera.org:8080/#/c/10748/5/impala-parent/pom.xml@148 PS5, Line 148: <id>impala.cdh.repo</id> > We should probably rename this id to something else. Perhaps "impala.cdh.re Done http://gerrit.cloudera.org:8080/#/c/10748/5/impala-parent/pom.xml@150 PS5, Line 150: <name>Impala CDH Repository</name> > rename this too Done http://gerrit.cloudera.org:8080/#/c/10748/5/testdata/pom.xml File testdata/pom.xml: http://gerrit.cloudera.org:8080/#/c/10748/5/testdata/pom.xml@39 PS5, Line 39: <!-- Force json-smart dependency. > Can you mention in your commit message what's up with json-smart and javax- Done -- To view, visit http://gerrit.cloudera.org:8080/10748 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I66c0dcb8abdd0d187490a761f129cda3b3500990 Gerrit-Change-Number: 10748 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Comment-Date: Thu, 21 Jun 2018 04:11:38 +0000 Gerrit-HasComments: Yes
