David Knupp has posted comments on this change. Change subject: Add a script to test performance on a developer machine ......................................................................
Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/6818/4/bin/single_node_perf_run.py File bin/single_node_perf_run.py: Line 71: os.chdir(IMPALA_HOME) Given that you os.chdir(IMPALA_HOME) in main(), can it be removed elsewhere? Line 241: parser = OptionParser() Nit: I usually like to refactor the boilerplate option-parsing stuff to another function that returns the "options, args" tuple, and then call that from the main function. Personal preference though. Line 274: if not 1 <= len(args) <= 2: Seems confusing that 1 git hash is allowed when the docstrings and usage all say "Compares the performance of Impala at two git hashes..." -- To view, visit http://gerrit.cloudera.org:8080/6818 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I70ba7f3c28f612a370915615600bf8dcebcedbc9 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-HasComments: Yes
