David Knupp has posted comments on this change.

Change subject: Enabling end-to-end tests on a remote cluster
......................................................................


Patch Set 2:

(11 comments)

Harrison, I'm addressing the comments on all the files other than 
remote_data_load.py. Since that's a new file, it won't have any effect on the 
current local workflow, and so can't introduce an regressions.

http://gerrit.cloudera.org:8080/#/c/4769/1/testdata/bin/compute-table-stats.sh
File testdata/bin/compute-table-stats.sh:

PS1, Line 27: IMPALAD
> Similar comment to below: do we want these defined globally in one spot, ma
IMPALA-4346 has been filed.


http://gerrit.cloudera.org:8080/#/c/4769/1/testdata/bin/create-load-data.sh
File testdata/bin/create-load-data.sh:

PS1, Line 38: HS2_HOST_PORT
> This is also defined in create-table-many-blocks.sh.  Is it not global, or 
I think the latter is preferable -- corral all the required configs in one 
place, and fail if not defined there. IMPALA-4346 was created to track the 
issue.


PS1, Line 259: 777
> Why not update with the super user?  Is this something that would affect th
IMPALA-4345 has been filed.


PS1, Line 303: REMOTE_LOAD
> Suggest comment why don't need this for REMOTE_LOAD.
IMPALA-4347 has been filed.


PS1, Line 374: if
> Indentation
Done


PS1, Line 375: read-only
> Why don't we need this for remote load?  If this is a temporary workaround,
IMPALA-4347 has been filed.


http://gerrit.cloudera.org:8080/#/c/4769/1/testdata/bin/load-test-warehouse-snapshot.sh
File testdata/bin/load-test-warehouse-snapshot.sh:

PS1, Line 75: 1777
> I think it's good to explicitly set the permissions, but I'd expect we may 
Good catch. This was causing one test to fail:

impala-py.test tests/custom_cluster/test_insert_behaviour.py -k 
test_insert_inherit_permission_disabled

Commenting this line out does not seem to affect dataload, or other tests. 
IMPALA-4345 filed to track the work of finding out why this was left here.


http://gerrit.cloudera.org:8080/#/c/4769/1/testdata/bin/load_nested.py
File testdata/bin/load_nested.py:

PS1, Line 302: parser.add_argument("-u", "--cm_user", default="admin")
             :   parser.add_argument("--cm_password", default="admin")
             :   parser.add_argument("--cm_host")
> When I look in tests/comparison/cli_options.py I see add_cm_options that mi
Done


http://gerrit.cloudera.org:8080/#/c/4769/1/testdata/bin/run-step.sh
File testdata/bin/run-step.sh:

PS1, Line 28: mkdir
> It seems odd to have a side affect on import given that this is essentially
Done


http://gerrit.cloudera.org:8080/#/c/4769/1/testdata/bin/setup-hdfs-env.sh
File testdata/bin/setup-hdfs-env.sh:

PS1, Line 40: REMOTE_LOAD
> I'd think we still have to create encryption keys for remote tests.  Is thi
IMPALA-4344 was filed.


PS1, Line 53: CACHEADMIN_ARGS
> Seems like with the previous setting in the is_kerberized block above, this
Clarification: can you be more explicit about the check you want? Something 
like: only run this block if CACHEADMIN_ARGS is still zero length?


-- 
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: 2
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: Michael Brown <[email protected]>
Gerrit-HasComments: Yes

Reply via email to