Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/22362 )
Change subject: IMPALA-13682: Implement missing capabilities in ImpylaHS2Client ...................................................................... Patch Set 9: (9 comments) http://gerrit.cloudera.org:8080/#/c/22362/9/tests/common/impala_connection.py File tests/common/impala_connection.py: http://gerrit.cloudera.org:8080/#/c/22362/9/tests/common/impala_connection.py@221 PS9, Line 221: pass nit: passed http://gerrit.cloudera.org:8080/#/c/22362/9/tests/common/impala_connection.py@298 PS9, Line 298: while (impala_state not in expected_impala_states It could be helpful to quite early if based on the current state the goal states cannot be reached, e.g. if the query is in ERROR state while we are waiting for RUNNING. http://gerrit.cloudera.org:8080/#/c/22362/9/tests/common/impala_connection.py@559 PS9, Line 559: try: why not include impyla.connect also in the try block? http://gerrit.cloudera.org:8080/#/c/22362/9/tests/common/impala_connection.py@622 PS9, Line 622: cursor = self.__open_single_cursor(user=user) maybe do some logging in this case? http://gerrit.cloudera.org:8080/#/c/22362/9/tests/common/impala_connection.py@754 PS9, Line 754: while (time.time() - start_time < timeout_s): return early in error state? http://gerrit.cloudera.org:8080/#/c/22362/9/tests/common/impala_test_suite.py File tests/common/impala_test_suite.py: http://gerrit.cloudera.org:8080/#/c/22362/9/tests/common/impala_test_suite.py@347 PS9, Line 347: cls shouldn't this be self? http://gerrit.cloudera.org:8080/#/c/22362/9/tests/common/impala_test_suite.py@351 PS9, Line 351: cls shouldn't this be self? http://gerrit.cloudera.org:8080/#/c/22362/9/tests/custom_cluster/test_admission_controller.py File tests/custom_cluster/test_admission_controller.py: http://gerrit.cloudera.org:8080/#/c/22362/9/tests/custom_cluster/test_admission_controller.py@1897 PS9, Line 1897: ImpalaHS2Error Couldn't we simply use Exception or have some wrapper to handle both hs2 and beeswax exceptions? http://gerrit.cloudera.org:8080/#/c/22362/9/tests/util/thrift_util.py File tests/util/thrift_util.py: http://gerrit.cloudera.org:8080/#/c/22362/9/tests/util/thrift_util.py@85 PS9, Line 85: query_id Isn't it session id? -- To view, visit http://gerrit.cloudera.org:8080/22362 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9ac07732424c16338e060c9392100b54337f11b8 Gerrit-Change-Number: 22362 Gerrit-PatchSet: 9 Gerrit-Owner: Riza Suminto <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Andrew Sherman <[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: Riza Suminto <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Wed, 12 Feb 2025 18:25:33 +0000 Gerrit-HasComments: Yes
