Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/22336 )

Change subject: IMPALA-13668: Add default_test_protocol parameter to py.test
......................................................................


Patch Set 1: Code-Review+1

(2 comments)

This makes sense to me

http://gerrit.cloudera.org:8080/#/c/22336/1/tests/conftest.py
File tests/conftest.py:

http://gerrit.cloudera.org:8080/#/c/22336/1/tests/conftest.py@57
PS1, Line 57: DEFAULT_TEST_PROTOCOL = 'beeswax'
If we made it possible to set this via an environment variable, it would make 
it easier to enable in a Jenkins job (e.g. like KUDU_MASTER_HOSTS above).


http://gerrit.cloudera.org:8080/#/c/22336/1/tests/custom_cluster/test_parquet_max_page_header.py
File tests/custom_cluster/test_parquet_max_page_header.py:

http://gerrit.cloudera.org:8080/#/c/22336/1/tests/custom_cluster/test_parquet_max_page_header.py@63
PS1, Line 63:     self.create_impala_clients()
Orthogonal to the change: When do we need to create a new client for customer 
cluster tests?

I see calls to create_impala_clients() for when we restart the coordinator, 
which makes sense. There are locations that run multiple threads and obviously 
need separate clients.

CustomClusterTestSuite inherits from ImpalaTestSuite, and ImpalaTestSuite 
initializes the clients in setup_class(). I guess some custom cluster tests are 
restarting per test case rather than per test suite. Maybe 
CustomClusterTestSuite should be calling create_impala_clients() in 
setup_method()?

This is not really related to your change. I'm just thinking about why this 
location exists and whether we should file a follow-up JIRA.



--
To view, visit http://gerrit.cloudera.org:8080/22336
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9165ea220b2c83ca36d6e68ef3b88b128310af23
Gerrit-Change-Number: 22336
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Jason Fehr <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Comment-Date: Thu, 23 Jan 2025 18:11:13 +0000
Gerrit-HasComments: Yes

Reply via email to