Nithya Janarthanan has posted comments on this change. ( http://gerrit.cloudera.org:8080/10100 )
Change subject: IMPALA-6923: Update scripts in benchmark folder to store workload and few minor updates ...................................................................... Patch Set 12: (5 comments) http://gerrit.cloudera.org:8080/#/c/10100/12/tests/benchmark/perf_result_datastore.py File tests/benchmark/perf_result_datastore.py: http://gerrit.cloudera.org:8080/#/c/10100/12/tests/benchmark/perf_result_datastore.py@23 PS12, Line 23: from subprocess import * > "*" is an antipattern for imports. It is better to be explicit about import Done Removed this import since it is not used in the file http://gerrit.cloudera.org:8080/#/c/10100/12/tests/benchmark/perf_result_datastore.py@63 PS12, Line 63: def insert_workload_summary(self, run_info_id): : self._insert_workload_summary(run_info_id) > This doesn't seem to do anything useful, so just rename _insert_workload_su Done. Good catch http://gerrit.cloudera.org:8080/#/c/10100/12/tests/benchmark/perf_result_datastore.py@85 PS12, Line 85: dont_save_profiles): > Negatives are weird. It would be better to call this save_profiles and flip Done http://gerrit.cloudera.org:8080/#/c/10100/12/tests/benchmark/perf_result_datastore.py@88 PS12, Line 88: temp_profile_folder = os.path.join(home_dir,"workspace","impala-workload-runner","profiles") > This is badly formated. There should be spaces between parameters. impala-f Done http://gerrit.cloudera.org:8080/#/c/10100/12/tests/benchmark/perf_result_datastore.py@155 PS12, Line 155: self._cache: Dictionary of with run_info/user as key/value > This isn't quite right. self._cache appears to be a hash in which a tuple o Done -- To view, visit http://gerrit.cloudera.org:8080/10100 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ica7c00ad59963d466bae9e607a4692af0138962c Gerrit-Change-Number: 10100 Gerrit-PatchSet: 12 Gerrit-Owner: Nithya Janarthanan <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Reviewer: Jinchul Kim <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-Reviewer: Mostafa Mokhtar <[email protected]> Gerrit-Reviewer: Nithya Janarthanan <[email protected]> Gerrit-Comment-Date: Fri, 20 Jul 2018 18:11:01 +0000 Gerrit-HasComments: Yes
