David Knupp has posted comments on this change. Change subject: IMPALA-3971, IMPALA-3229: Bootstrap an Impala dev environment ......................................................................
Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/4674/2/bin/bootstrap_development.sh File bin/bootstrap_development.sh: Line 20: # This script bootstraps a development environment from almost So, this first comment may be a nit, but it would be nice to clarify why this script came to be added. We have a various scripts that float around /bin and /testdata/bin that sometimes seem to have overlapping functions (e.g., there are multiple entry points for running tests), and it's not always immediately clear why they all coexist. This script consolidates some needed steps, such as cloning and running the chef setup, and building Impala, but in so doing, it also makes implicit assumptions by hard-coding in the command line args to buildall.sh for bypass the test warehouse snaphot, and running all tests after the build process completes. If nothing else, it might be nice to explicitly state who the intended users/audience for this script might be, and why these assumptions are being made. Line 31: echo "${HOMEDIR} is needed for installing Impala dependencies" This is really an artificial limitation/oversight of the chef scripts. Would it make sense to revisit those first, rather than propagating it to another script? PS2, Line 70: 999999 999999 was used as an arbitrarily large number in the original one-off Jenkins job. But maybe there's a way to truly disable the max pytest failures, e.g., some tools do this by specifying -1 for values like this? PS2, Line 72: -skiptests What is the difference between passing in --skiptests and then calling run-all-tests.sh separately, versus simply leaving off --skiptests? -- To view, visit http://gerrit.cloudera.org:8080/4674 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If166a8a286d7559af547da39f6cc09e723f34c7e Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Taras Bobrovytsky <[email protected]> Gerrit-HasComments: Yes
