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 12: (1 comment) http://gerrit.cloudera.org:8080/#/c/23385/12//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/23385/12//COMMIT_MSG@12 PS12, Line 12: 1. Supporting code for asserting traces was moved to > I guess I don't understand the rationale for moving it to a utility, and wh The code to assert traces is 439 lines long. Because python requires class instance functions be declared before they are used, all those lines came before the custom cluster tests in test_otel_trace.py. It made locating the actual tests themselves difficult and the entire file was very difficult to understand since it has 1,176 lines. Moving the code to assert traces is not strictly necessary, but it makes the code much more maintainable and comprehendable. Since the code to assert traces was originally part of the Otel test base class, it had access to all methods on CustomClusterTestSuite instances. When the code moved outside that class into otel_trace.py, it was no longer part of any class and thus lost access to all CustomClusterTestSuite functions and could no longer call assert_impalad_log_contains -- 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: 12 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-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Sat, 13 Sep 2025 02:23:00 +0000 Gerrit-HasComments: Yes
