Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/22358 )
Change subject: IMPALA-13672: Migrate query_test/test_kudu.py to use hs2 protocol ...................................................................... Patch Set 4: (5 comments) http://gerrit.cloudera.org:8080/#/c/22358/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/22358/4//COMMIT_MSG@12 PS4, Line 12: - Remove usage of deprecated cursor and unique_cursor fixture. it could be noted that these used hs2 protocol, which means that some tests (without vector) are actually moved from hs2 to beeswax in the patch http://gerrit.cloudera.org:8080/#/c/22358/4//COMMIT_MSG@27 PS4, Line 27: Added convenience method execute_query_using_vector() and : create_impala_client_from_vector(). : : After this patch, all test_kudu.py tests that accept 'vector' fixture : will effectively use hs2 protocol. Test that does not accept 'vector' : will continue to use ImpalaTestSuite.cient that is a beeswax client. If we always use hs2 within the test_suite, couldn't we ensure in setup_class that execute_query() uses hs2_client, e.g. by cls.client = cls.hs2_client? Or the goal in long term is to be able to use whatever the protocol is in the vector? http://gerrit.cloudera.org:8080/#/c/22358/4/tests/common/impala_connection.py File tests/common/impala_connection.py: http://gerrit.cloudera.org:8080/#/c/22358/4/tests/common/impala_connection.py@566 PS4, Line 566: self.profile = profile : # ImpalaBeeswaxResult store profile at runtime_profile field : self.runtime_profile = profile Can't we use a single member in both? http://gerrit.cloudera.org:8080/#/c/22358/4/tests/common/impala_test_suite.py File tests/common/impala_test_suite.py: http://gerrit.cloudera.org:8080/#/c/22358/4/tests/common/impala_test_suite.py@340 PS4, Line 340: p nit: +2 indentation http://gerrit.cloudera.org:8080/#/c/22358/4/tests/query_test/test_kudu.py File tests/query_test/test_kudu.py: http://gerrit.cloudera.org:8080/#/c/22358/4/tests/query_test/test_kudu.py@173 PS4, Line 173: kudu_client.latest_observed_timestamp())) nit: +2 indentation - this files seems quite inconsistent about indentation in broken lines after the change -- To view, visit http://gerrit.cloudera.org:8080/22358 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5f38baf5a0bbde1a1ad0bb4666c300f4f3cabd33 Gerrit-Change-Number: 22358 Gerrit-PatchSet: 4 Gerrit-Owner: Riza Suminto <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Comment-Date: Sat, 25 Jan 2025 16:35:13 +0000 Gerrit-HasComments: Yes
