impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "David Knupp (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] Enabling end-to-end tests on a remote cluster
Date Sat, 22 Oct 2016 00:47:04 GMT
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 <dknupp@cloudera.com>
Gerrit-Reviewer: David Knupp <dknupp@cloudera.com>
Gerrit-Reviewer: Harrison Sheinblatt <hs7@hotmail.com>
Gerrit-Reviewer: Michael Brown <mikeb@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message