Skip to content

Conversation

@sigmavirus24
Copy link
Contributor

Fixes #1995

@kevinburke
Copy link
Contributor

+1

@t-8ch
Copy link
Contributor

t-8ch commented Aug 29, 2014

Could we use another name which doesn't sound like it will solve all problems?

@dstufft
Copy link
Contributor

dstufft commented Aug 29, 2014

So, I'm going to raise a question!

I was the one who opened the original ticket, but I wonder if it makes sense to do this anymore. On Python 3.4+ and Python 2.7.9+ it's not better anymore. On Python 3.2 and 3.3 it's better, but not in ways that couldn't be reproduced without extra dependencies.

Either way, this PR looks good to me, the only question is if we still need/want it just for Python 2.6 and Python <=2.7.8.

@sigmavirus24
Copy link
Contributor Author

I don't think it will hurt to have it for 2.6 or <= 2.7.8.

@Lukasa
Copy link
Member

Lukasa commented Aug 30, 2014

I agree that it won't hurt us to have, but I agree with @t-8ch that we should avoid the slightly prejudicial name. Maybe just call it pyopenssl?

@sigmavirus24
Copy link
Contributor Author

I really have no opinion on the name. I would think with_sni (i.e., pip install requests[with_sni] reads well) would be most descriptive of the use case for the largest number of users, but pyopenssl is fine too. I'll wait for @kennethreitz to decide.

@kennethreitz
Copy link
Contributor

I'd rather give it a more generic/rememberable name.

security

kennethreitz added a commit that referenced this pull request Sep 4, 2014
@kennethreitz kennethreitz merged commit bc25604 into psf:master Sep 4, 2014
@kennethreitz
Copy link
Contributor

Updated to security. Now, let's document it!

@t-8ch
Copy link
Contributor

t-8ch commented Sep 4, 2014

I fear this generic and positive name will lead to people enabling it that don't need it. Triggering compilation of pyopenssl, breakage when missing development tools etc. And on Python3 it will be ignored anyways.

@sigmavirus24 sigmavirus24 deleted the betterssl branch September 4, 2014 19:18
@kennethreitz
Copy link
Contributor

@t-8ch false

@t-8ch
Copy link
Contributor

t-8ch commented Sep 5, 2014

urllib3 uses ndg-httpsclient. Importing this on py3 will raise a syntax error
and abort the import -> PyOpenSSL is not used.
In requests < 2.4.0 The syntax error will even bubble up and just crash
everything.

@kennethreitz
Copy link
Contributor

we can fix that in setup.py

@t-8ch
Copy link
Contributor

t-8ch commented Sep 5, 2014

You mean: requests[betterssl] on Py3 will just be a noop?

@sigmavirus24
Copy link
Contributor Author

@t-8ch I don't understand your concerns in this statement:

In requests < 2.4.0 The syntax error will even bubble up and just crash everything.

The extra does not ship with anything <= 2.4.0. How does this affect those people?

@t-8ch
Copy link
Contributor

t-8ch commented Sep 5, 2014

Just an example for downsides which can arise when using the pyopenssl backend

bcb referenced this pull request in explodinglabs/jsonrpcclient Sep 22, 2016
Python versions prior to 2.7.9 should install with pip install
'jsonrpcclient[requests_security]'. See requests issue
https://github.com/kennethreitz/requests/issues/1995 resolved in PR
https://github.com/kennethreitz/requests/pull/2195

Closes #31
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create an Extra for Better SSL Support

6 participants