Jason Fehr has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23385 )

Change subject: IMPALA-14401: Deflake/Improve OpenTelemetry Tracing Tests
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/23385/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/23385/2//COMMIT_MSG@22
PS2, Line 22:    contained a total of 11 individual tests that used the
            :    'unique_database' fixture. When this fixture is used in a 
test, it
            :    results in two DDLs being run before the test to drop/create 
the
            :    database and one DDL being run after the test to drop the
> Consider do this in the TestOtelTraceBase instead of on each individual tes
I tried moving db create/drop to TestOtelTraceBase, but I ran into a problem 
with the "TestOtelTraceSelectQueued" class.  This class has the 
@CustomClusterTestSuite annotation on each test method since each test uses its 
own startup flags.  Since those annotations are on the test methods, there is 
no client available during the setup_class function.

Additionally, the setup_class function runs once per subclass, which means 
moving the db create/drop to TestOtelTraceBase will end up creating/dropping 
more databases than necessary.  Thus, I left the setup/teardown class methods 
where they were.

The setup/teardown class methods in CustomClusterTestSuite start up and shut 
down the test Impala cluster.  There will not be flakiness from overriding 
setup_class and teardown_class since TestOtelTraceBase subclass code calls 
those functions at the appropriate time (startup happens before this class's 
code, and teardown happens after this class's code).


http://gerrit.cloudera.org:8080/#/c/23385/2/tests/util/otel_trace.py
File tests/util/otel_trace.py:

http://gerrit.cloudera.org:8080/#/c/23385/2/tests/util/otel_trace.py@353
PS2, Line 353:   db_user = parse_db_user(query_profile)
> So far, all CustomClusterTest implementations are put in either authorizati
This is a very good point that I agree with about which code should be in util 
files.  I reorganized the code some so that code in this file is provided all 
the data it needs to assert a trace.  The "TestOtelTrace" class is in 
test_otel_trace.py and has been renamed to "TestOtelTraceBase"



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c3e0075df688c7ae601c6f2e5743f56d6db100e
Gerrit-Change-Number: 23385
Gerrit-PatchSet: 4
Gerrit-Owner: Jason Fehr <[email protected]>
Gerrit-Reviewer: Abhishek Rawat <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Jason Fehr <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Comment-Date: Mon, 08 Sep 2025 22:59:29 +0000
Gerrit-HasComments: Yes

Reply via email to