You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2018/12/28 00:55:21 UTC
[kudu-CR] [python] handle DisableOpenSSLInitialization() result status
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12135
Change subject: [python] handle DisableOpenSSLInitialization() result status
......................................................................
[python] handle DisableOpenSSLInitialization() result status
This patch adds verification for DisableOpenSSLInitialization().
The motivation for this change is two-fold:
a) avoid glitches while initializing the OpenSSL library
by python Kudu client if DisableOpenSSLInitialization()
de facto failed
b) get rid of compilation warnings on unhandled return value
of the DisableOpenSSLInitialization() function
Change-Id: I1963b6d87d731fbfa87a09b986595aa8ea00da60
---
M python/kudu/client.pyx
1 file changed, 4 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/12135/1
--
To view, visit http://gerrit.cloudera.org:8080/12135
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1963b6d87d731fbfa87a09b986595aa8ea00da60
Gerrit-Change-Number: 12135
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [python] handle DisableOpenSSLInitialization() result status
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12135 )
Change subject: [python] handle DisableOpenSSLInitialization() result status
......................................................................
[python] handle DisableOpenSSLInitialization() result status
This patch adds verification for DisableOpenSSLInitialization().
The motivation for this change is two-fold:
a) avoid glitches while initializing the OpenSSL library
by python Kudu client if DisableOpenSSLInitialization()
de facto failed
b) get rid of compilation warnings on unhandled return value
of the DisableOpenSSLInitialization() function
Change-Id: I1963b6d87d731fbfa87a09b986595aa8ea00da60
Reviewed-on: http://gerrit.cloudera.org:8080/12135
Tested-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M python/kudu/client.pyx
1 file changed, 4 insertions(+), 3 deletions(-)
Approvals:
Alexey Serbin: Verified
Adar Dembo: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/12135
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1963b6d87d731fbfa87a09b986595aa8ea00da60
Gerrit-Change-Number: 12135
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [python] handle DisableOpenSSLInitialization() result status
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12135 )
Change subject: [python] handle DisableOpenSSLInitialization() result status
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12135/1/python/kudu/client.pyx
File python/kudu/client.pyx:
http://gerrit.cloudera.org:8080/#/c/12135/1/python/kudu/client.pyx@288
PS1, Line 288: # Python programs will often have already imported _ssl, which
: # has the side effect of initializing OpenSSL. So, we detect
: # whether _ssl is present, and if we can import it, we disable
: # Kudu's initialization to avoid a conflict.
Just to clarify, "import _ssl" is not the "correct" way to initialize OpenSSL in Python, and it will fail if the correct way (whatever it may be) is not followed? Asking because it seems unusual to expect an import statement to fail in Python (unless it's for an optional module), so I want to make sure I understand the behavior here.
--
To view, visit http://gerrit.cloudera.org:8080/12135
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1963b6d87d731fbfa87a09b986595aa8ea00da60
Gerrit-Change-Number: 12135
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Comment-Date: Wed, 02 Jan 2019 17:50:25 +0000
Gerrit-HasComments: Yes
[kudu-CR] [python] handle DisableOpenSSLInitialization() result status
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/12135 )
Change subject: [python] handle DisableOpenSSLInitialization() result status
......................................................................
Removed reviewer Kudu Jenkins with the following votes:
* Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/12135
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I1963b6d87d731fbfa87a09b986595aa8ea00da60
Gerrit-Change-Number: 12135
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [python] handle DisableOpenSSLInitialization() result status
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12135 )
Change subject: [python] handle DisableOpenSSLInitialization() result status
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12135/1/python/kudu/client.pyx
File python/kudu/client.pyx:
http://gerrit.cloudera.org:8080/#/c/12135/1/python/kudu/client.pyx@288
PS1, Line 288: # Python programs will often have already imported _ssl, which
: # has the side effect of initializing OpenSSL. So, we detect
: # whether _ssl is present, and if we can import it, we disable
: # Kudu's initialization to avoid a conflict.
> Just to clarify, "import _ssl" is not the "correct" way to initialize OpenS
Yep, "import _ssl" is not the "standard" way of getting SSL/TLS socket support in Python. The "standard" way is to call "import ssl". The 'ssl' Python's module is a wrapper around the '_ssl' module; the 'ssl module calls "import _ssl" by itself. Essentially, the 'ssl' module provides SSLContext, SSLSocket and a few other wrapper classes, plus handy utility functions.
The "import _ssl" will fail if OpenSSL is not present or the Python interpreter is built without TLS/SSL support. The "import _ssl" statement is about initting the OpenSSL library and importing "low-level" functionality for TLS/SSL. As I understand, the original idea behind this try/except was to unconditionally disable initting of the OpenSSL library by the underlying Kudu C++ client if Python is build with TLS/SSL support.
Kudu requires that the OpenSSL library is initialized with support for multi-threading, and the DisableOpenSSLInitialization() function checks for that behind the scenes.
As an additional info, calling DisableOpenSSLInitialization() prior to initting the OpenSSL library would result in non-OK result status from DisableOpenSSLInitialization(). Calling DisableOpenSSLInitialization() on not-multi-thread-compatible-initialized OpenSSL library would result in non-OK result status as well. This small patch is targeted to handle the latter case (if it ever happens) and to remove the compilation warning.
--
To view, visit http://gerrit.cloudera.org:8080/12135
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1963b6d87d731fbfa87a09b986595aa8ea00da60
Gerrit-Change-Number: 12135
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Comment-Date: Wed, 02 Jan 2019 20:16:55 +0000
Gerrit-HasComments: Yes
[kudu-CR] [python] handle DisableOpenSSLInitialization() result status
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12135 )
Change subject: [python] handle DisableOpenSSLInitialization() result status
......................................................................
Patch Set 1: Verified+1
Unrelated flake in DenseNodeTest.RunTest
--
To view, visit http://gerrit.cloudera.org:8080/12135
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1963b6d87d731fbfa87a09b986595aa8ea00da60
Gerrit-Change-Number: 12135
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 28 Dec 2018 03:59:54 +0000
Gerrit-HasComments: No