Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10529 )

Change subject: IMPALA-6990: TestClientSsl.test_tls_v12 failing due to Python 
SSL error
......................................................................


Patch Set 4: Code-Review+2

(5 comments)

Thanks for the reviews! Upgrading to +2 since 2 reviewers +1'd it.

http://gerrit.cloudera.org:8080/#/c/10529/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10529/3//COMMIT_MSG@9
PS3, Line 9: When we upgraded to thrift-0.9.3, the TSSLSocket.py logic changed 
quite
> To be super clear: we used to ignore this silently but now TSSLSocket.py br
I explained this in a previous top level comment.


http://gerrit.cloudera.org:8080/#/c/10529/3//COMMIT_MSG@13
PS3, Line 13: 
https://github.com/apache/thrift/blob/88591e32e710a0524327153c8b629d5b461e35e0/lib/py/src/transport/TSSLSocket.py#L37-L41
> You can press "y" in the github UI to get code a permalink UI. Or specify a
Done


http://gerrit.cloudera.org:8080/#/c/10529/3/tests/custom_cluster/test_client_ssl.py
File tests/custom_cluster/test_client_ssl.py:

http://gerrit.cloudera.org:8080/#/c/10529/3/tests/custom_cluster/test_client_ssl.py@36
PS3, Line 36: HAS_LEGACY_OPENSSL = getattr(ssl, "OPENSSL_VERSION_NUMBER", None)
> It's super confusing that you'd change a constant on line 43. Can you just
Done


http://gerrit.cloudera.org:8080/#/c/10529/3/tests/custom_cluster/test_client_ssl.py@45
PS3, Line 45:
> If you want, you can use "getattr" with a default to avoid this.
Done


http://gerrit.cloudera.org:8080/#/c/10529/3/tests/custom_cluster/test_client_ssl.py@132
PS3, Line 132:                                     
statestored_args=SSL_WILDCARD_ARGS,
> Consider just inlining
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I92c66ecaeb94b0c83ee6f1396c082709c21b3187
Gerrit-Change-Number: 10529
Gerrit-PatchSet: 4
Gerrit-Owner: Sailesh Mukil <[email protected]>
Gerrit-Reviewer: Michael Brown <[email protected]>
Gerrit-Reviewer: Philip Zeyliger <[email protected]>
Gerrit-Reviewer: Sailesh Mukil <[email protected]>
Gerrit-Comment-Date: Thu, 31 May 2018 02:17:22 +0000
Gerrit-HasComments: Yes

Reply via email to