David Knupp has posted comments on this change. (
http://gerrit.cloudera.org:8080/13386 )
Change subject: IMPALA-8553,IMPALA-8552: fix checks for remote cluster
......................................................................
Patch Set 6:
Sorry -- it took me a while to get back to this. It's been so long enough that
we both probably lost the context. :-(
Anyway, I'm not sure that this is really a substantial improvement, but one
could do something like this, and it would probably be considered somewhat
more typical.
@pytest.yield_fixture(scope="session", autouse = True)
def cluster_environment():
"""Set up test cluster properties for the test session"""
cluster_environment = ImpalaTestClusterProperties.get_instance()
yield cluster_environment
Then you wouldn't need to import anything in the individual test modules, or
call get_instance() in multiple places -- you could just use the
cluster_environment test fixture where needed, e.g.:
# metadata/test_hms_integration.py
@pytest.mark.execute_serially
def test_sanity(self, vector, cluster_environment):
# Create a database in Hive
self.run_stmt_in_hive("drop database if exists hms_sanity_db cascade")
self.run_stmt_in_hive("create database hms_sanity_db")
if cluster_environment.is_catalog_v2_cluster():
self.wait_for_db_to_appear("hms_sanity_db", timeout_s=30)
# etc...
I ran this against a remote cluster, and it worked.
Not sure how much that buys you though. It's a little cleaner. But you're right
-- the line between python and pytest code in our code base is fuzzy in places,
e.g. in start-impala-minicluster.py, and even skip.py (which is sort of general
python library code, and sort of pytest code in that it expects pytest.config
to be populated).
Anyway, if you want to keep the original version you've already submitted, I
think that would be fine. It's an improvement over the mess that exists. If you
find value in setting up an autouse session-scoped fixture, that would be fine
as well.
--
To view, visit http://gerrit.cloudera.org:8080/13386
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa6b2a1391f53121d3d7c00c5cf0a57590899ce4
Gerrit-Change-Number: 13386
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: David Knupp <dkn...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 00:28:57 +0000
Gerrit-HasComments: No
