Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/15990 )
Change subject: IMPALA-9673: Add external warehouse dir variable in E2E test ...................................................................... Patch Set 7: Code-Review+1 (3 comments) This looks good to me. Just a couple nits. I will bump to +2 when those are fixed. http://gerrit.cloudera.org:8080/#/c/15990/7/bin/impala-config.sh File bin/impala-config.sh: http://gerrit.cloudera.org:8080/#/c/15990/7/bin/impala-config.sh@181 PS7, Line 181: export CDP_RANGER_VERSION=2.0.0.7.2.1.0-57 : export CDP_TEZ_VERSION=0.9.1.7.2.1.0-57 : export CDP_KNOX_VERSION=1.3.0.7.2.1.0-57 : export CDP_OZONE_VERSION=0.6.0.7.2.1.0-57 Nit: I missed this in previous rounds, but please leave the components in alphabetical order (i.e. same order as before). http://gerrit.cloudera.org:8080/#/c/15990/7/tests/query_test/test_compressed_formats.py File tests/query_test/test_compressed_formats.py: http://gerrit.cloudera.org:8080/#/c/15990/7/tests/query_test/test_compressed_formats.py@74 PS7, Line 74: def test_compressed_formats(self, vector): Just in case you haven't seen this before, we have a variant on the test function signature that adds a "unique_database" parameter. i.e. def test_compressed_format(self, vector, unique_database): That fixture creates a unique database before the test runs and drops it afterward. This allows test writers to put tables inside the unique database and not worry about concurrent modifications or naming conflicts. This test is fine either way, but I wanted to make sure you knew about it. http://gerrit.cloudera.org:8080/#/c/15990/7/tests/query_test/test_compressed_formats.py@102 PS7, Line 102: # Since Hive makes tables managed by default (only matters on Hive 3+), it : # will use the managed warehouse location. Nit: Update this comment -- To view, visit http://gerrit.cloudera.org:8080/15990 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I57926babf4caebfd365e6be65a399f12ea68687f Gerrit-Change-Number: 15990 Gerrit-PatchSet: 7 Gerrit-Owner: Xiaomeng Zhang <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Sahil Takiar <[email protected]> Gerrit-Reviewer: Xiaomeng Zhang <[email protected]> Gerrit-Comment-Date: Wed, 03 Jun 2020 01:36:04 +0000 Gerrit-HasComments: Yes
