Tim Armstrong has posted comments on this change. Change subject: Guide to important environment variables for build, test, and mini-cluster operations. ......................................................................
Patch Set 2: (7 comments) Looks good to me. I noticed a few more things we could clean up but we don't necessarily need to bundle them in this change. http://gerrit.cloudera.org:8080/#/c/7350/2/bin/impala-config.sh File bin/impala-config.sh: PS2, Line 337: Nice catch Line 460 Thank you for removing this. Line 343: if type nproc >/dev/null 2>&1; then I used "getconf _NPROCESSORS_ONLN" in infra/python/bootstrap_virtualenv.py. My understanding is that it works pretty much anywhere so we could avoid this branch.. Tangential but might as well simplify this script while we're here. PS2, Line 433: nproc Hmm, should this use $CORES? Line 456: LIBHDFS_OPTS="${LIBHDFS_OPTS}:${IMPALA_HOME}/be/build/debug/service" Pretty sure we don't need this, since everything works on a release build. It's kind of tangential but while we're here... Line 473: LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:${IMPALA_HOME}/be/build/debug/service" Same here. Pretty sure that we don't need to add this to LD_LIBRARY_PATH. Line 491: alias gerrit-verify-only="${IMPALA_AUX_TEST_HOME}/jenkins/gerrit-verify-only.sh" We can remove this while we're here too - it's referring to a Cloudera-internal script which a) doesn't belong in the ASF repo and b) no longer works since we moved to jenkins.impala.io -- To view, visit http://gerrit.cloudera.org:8080/7350 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I16d34cb4fa0c60c5ad6d9c8764cc0ec21c5cb368 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zach Amsden <[email protected]> Gerrit-HasComments: Yes
