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

Further improvements to pyopenssl sendall() #628

Merged
merged 3 commits into from
May 18, 2015
Merged

Further improvements to pyopenssl sendall() #628

merged 3 commits into from
May 18, 2015

Conversation

darkrain42
Copy link
Contributor

Trying to address the feedback from #626 and #627

  • no semi-magic number
  • use a memoryview, even if PyOpenSSL will still perform a conversion to string (to make cffi happy) today. Futureproofing!

This doesn't currently do anything useful, because PyOpenSSL needs to
convert back to a string to pass to ffi. It doesn't hurt, though.
@@ -204,10 +207,12 @@ def _send_until_done(self, data):
continue

def sendall(self, data):
if sys.version_info >= (2, 7) and not isinstance(data, memoryview):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be better to check for the presence of memoryview rather than check sys.version_info.

Maybe something like this near the top?

try:
    _ = memoryview
    has_memoryview = True
except NameError:
    has_memoryview = False

shazow added a commit that referenced this pull request May 18, 2015
Further improvements to pyopenssl sendall()
@shazow shazow merged commit 5f3fce4 into urllib3:master May 18, 2015
@shazow
Copy link
Member

shazow commented May 18, 2015

Thanks for the improvements, I appreciate you following up on this. :) Btw you're welcome to add yourself to our CONTRIBUTORS.txt file!

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

Successfully merging this pull request may close these issues.

None yet

2 participants