David Knupp has posted comments on this change. Change subject: IMPALA-4365: Enabling end-to-end tests on a remote cluster ......................................................................
Patch Set 9: (8 comments) Addressed comments. Also removed another place in remote_data_load.py where sys.exit() was called. Finally, fixed a bug that I noticed in the arg parsing code. http://gerrit.cloudera.org:8080/#/c/4769/9/bin/remote_data_load.py File bin/remote_data_load.py: PS9, Line 63: 'HDFS', : 'YARN', : 'HIVE', : 'IMPALA', : 'MAPREDUCE', : 'KUDU', : 'HBASE', : 'ZOOKEEPER > I think it would be nice to put these in sorted order. (Vim has a nice abil Done Line 174: try: > It's kind of weird to do this through try and except. How about: Done PS9, Line 177: missing_svcs > I suggest not shortening: missing_services Done Line 179: sys.exit("Cluster not ready.") > It's better to raise an exception that exiting right away. It might be unex Done Line 181: try: > it's better to do this with if then instead of assert. (same as above) Done PS9, Line 253: svc_type > I prefer service_type here and elsewhere. It's clearer and easier on the br Done Line 379: # 'database_name' is a header from the beeline output. > Maybe we can skip the header some other way? for example db.list.split()[1: Done http://gerrit.cloudera.org:8080/#/c/4769/9/testdata/bin/create-load-data.sh File testdata/bin/create-load-data.sh: Line 482: if [[ -n "$CM_HOST" ]]; then > this should be moved right above where it's used (line 491) Done -- To view, visit http://gerrit.cloudera.org:8080/4769 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1f443a1728a1d28168090c6f54e82dec2cb073e9 Gerrit-PatchSet: 9 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Harrison Sheinblatt <[email protected]> Gerrit-Reviewer: Martin Grund <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-Reviewer: Taras Bobrovytsky <[email protected]> Gerrit-HasComments: Yes
