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

Change subject: IMPALA-14776 (part 1): Auto-close manually created impala 
clients
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/24026/5/tests/custom_cluster/test_scratch_disk.py
File tests/custom_cluster/test_scratch_disk.py:

http://gerrit.cloudera.org:8080/#/c/24026/5/tests/custom_cluster/test_scratch_disk.py@97
PS5, Line 97: ImpalaTestSuite
> Maybe it would be better to have a base class like ManualCustomCluster test
It's a bit weird to bypass the parent method and go to the grandparent method. 
I wasn't sure what would be cleaner. I was thinking about adding a decorator 
like "with_args" (or maybe a special arg to with_args) to tell 
CustomClusterTestSuite not to start up the cluster.

Another thing I saw while looking at this:
There are a bunch of setup_class() methods that are either pointless (they just 
call super) or only exist to skip if it isn't exhaustive. We don't need the 
first set. We could avoid the second set by using 
SkipIfExploration.is_not_exhaustive(). Fewer custom setup_class() methods is 
probably a good thing.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib550527838a81cd2aaf69bb715080f6ac6da3786
Gerrit-Change-Number: 24026
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Jason Fehr <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Comment-Date: Wed, 04 Mar 2026 08:08:52 +0000
Gerrit-HasComments: Yes

Reply via email to