Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/23499 )
Change subject: IMPALA-14460: Implement connect_timeout_ms for HS2-HTTP ...................................................................... Patch Set 27: (2 comments) http://gerrit.cloudera.org:8080/#/c/23499/26/shell/impala_shell/impala_client.py File shell/impala_shell/impala_client.py: http://gerrit.cloudera.org:8080/#/c/23499/26/shell/impala_shell/impala_client.py@478 PS26, Line 478: transportImpl = transport : transport = TBufferedTransport(transport) > Wouldn't it make more sense to move this inside ImpalaHttpClient.open()? Th This effectively happens in ImpalaHttpClient.open() because this patch changed it to call connect() there. I opted to do it here because it matches _get_transport, where we setTimeout as soon as the TCP connection is established. We also needed access to the underling transport impl, which is masked after wrapping it in TBufferedTransport. Moving it later would keep the connect_timeout_ms configuration through establishing a session, which differs from how it's handled for the non-HTTP protocols. http://gerrit.cloudera.org:8080/#/c/23499/26/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/23499/26/tests/shell/test_shell_commandline.py@1405 PS26, Line 1405: def test_http_socket_timeout(self, vector): > This was an existing test gap, but it would be better to also test this wit That seems to be a general gap, we only seem to test ssl in custom_cluster/test_client_ssl.py. -- To view, visit http://gerrit.cloudera.org:8080/23499 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9012066fb0d16497f309532021d7b323404b9fb2 Gerrit-Change-Number: 23499 Gerrit-PatchSet: 27 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Pranav Lodha <[email protected]> Gerrit-Comment-Date: Thu, 08 Jan 2026 19:38:59 +0000 Gerrit-HasComments: Yes
