Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes for the CLI (SSL ThriftBackend) #3673

Open
wants to merge 2 commits into
base: stable
Choose a base branch
from
Open

Conversation

snilt
Copy link

@snilt snilt commented Jun 23, 2020

The ThriftClient doesn't work with current versions of pyOpenSSL.
This could possibly be related to: pyca/pyopenssl#168

The PR adds a timeout to the SecureSocketConnection class, which should work for any pyOpenSSL version, even the ancient not-cffi-based versions.

Since ThriftClient auto-detects if a server uses SSL, the socket timeout values are kept low to speed up the detection. This possibly can lead to problems on high latency network connections, the PR adds a command line option to the CLI to bypass the detection and use a higher timeout.

Tested on:
Debian Stretch, pyOpenSSL 16.2.0
Debian Jessie, pyOpenSSL 0.14
Windows 10, pyOpenSSL 19.1.0
Windows 10, pyOpenSSL 0.13.16 (egenix)

@pep8speaks
Copy link

pep8speaks commented Jun 23, 2020

Hello @snilt! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 41:17: E265 block comment should start with '# '
Line 52:17: E265 block comment should start with '# '
Line 111:80: E501 line too long (102 > 79 characters)
Line 119:80: E501 line too long (81 > 79 characters)
Line 124:80: E501 line too long (111 > 79 characters)

Line 35:80: E501 line too long (86 > 79 characters)
Line 41:80: E501 line too long (93 > 79 characters)
Line 43:80: E501 line too long (87 > 79 characters)
Line 47:80: E501 line too long (88 > 79 characters)
Line 74:80: E501 line too long (88 > 79 characters)
Line 78:17: E265 block comment should start with '# '
Line 78:80: E501 line too long (124 > 79 characters)
Line 92:80: E501 line too long (81 > 79 characters)
Line 109:80: E501 line too long (88 > 79 characters)

Line 478:80: E501 line too long (83 > 79 characters)
Line 505:80: E501 line too long (115 > 79 characters)
Line 552:80: E501 line too long (105 > 79 characters)
Line 560:80: E501 line too long (101 > 79 characters)
Line 561:80: E501 line too long (102 > 79 characters)
Line 569:33: E701 multiple statements on one line (colon)
Line 569:80: E501 line too long (82 > 79 characters)
Line 577:80: E501 line too long (109 > 79 characters)
Line 581:80: E501 line too long (82 > 79 characters)
Line 587:80: E501 line too long (105 > 79 characters)

Comment last updated at 2020-07-13 17:07:12 UTC

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@vuolter
Copy link
Contributor

vuolter commented Jul 13, 2020

Codacy Here is an overview of what got changed by this pull request:

Clones added
============
- module/remote/thriftbackend/Socket.py  1
- module/remote/thriftbackend/ThriftClient.py  2
         

See the complete overview on Codacy

@GammaC0de
Copy link
Member

The ThriftClient doesn't work with current versions of pyOpenSSL.
That is related to: pyca/pyopenssl#168

Can you give a brief explanation about this issue?

@snilt
Copy link
Author

snilt commented Jul 15, 2020

With ssl thriftbackend enabled on the server, when i start the cli, it hangs during connecting to server.

(About the pyopenssl issue I'm not sure if it is related. First post edited)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants