Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23519 )

Change subject: IMPALA-14452: Fix impala-shell SSL with Python 3.12
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/23519/1/shell/impala_shell/ImpalaHttpClient.py
File shell/impala_shell/ImpalaHttpClient.py:

http://gerrit.cloudera.org:8080/#/c/23519/1/shell/impala_shell/ImpalaHttpClient.py@75
PS1, Line 75:       self.port = parsed.port or http_client.HTTPS_PORT
            :       self.context = ssl_context
> Is it ok for ssl_context to be None? If not, an assert could check this
This behavior would be unchanged from before, if they passed cafile=None and 
ssl_context=None.

Based on 
https://docs.python.org/3/library/http.client.html#http.client.HTTPSConnection, 
context=None depends on the Python version but recent versions default to 
certificate and hostname checks using system CAs.


http://gerrit.cloudera.org:8080/#/c/23519/1/shell/impala_shell/TSSLSocketWithFixes.py
File shell/impala_shell/TSSLSocketWithFixes.py:

http://gerrit.cloudera.org:8080/#/c/23519/1/shell/impala_shell/TSSLSocketWithFixes.py@28
PS1, Line 28:
> Need to figure out what to do for a function that expects two arguments and
Done


http://gerrit.cloudera.org:8080/#/c/23519/1/shell/impala_shell/TSSLSocketWithFixes.py@32
PS1, Line 32:   This is a subclass of Thrift 0.16.0's TSSLSocket
> Can you add a link to the original TSSLSocket this matches with? https://gi
Done


http://gerrit.cloudera.org:8080/#/c/23519/1/shell/impala_shell/TSSLSocketWithFixes.py@42
PS1, Line 42:     # Earlier versions continue to use ssl.match_hostname.
            :     validate_callback = openssl_fallback if sys.version_info[0] > 
3 or (
            :         sys.version_info[0] == 3 and sys.version_info[1] >= 12) 
else None
            :     # Set client protocol choice to be very permissive, as we 
rely on servers to enforce
            :     # good protocol sele
> I would remove the defaults that are not used in impala-shell
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I046a9010ac4cb1f7d705935054b306cddaf8bdc7
Gerrit-Change-Number: 23519
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Comment-Date: Thu, 09 Oct 2025 17:27:21 +0000
Gerrit-HasComments: Yes

Reply via email to