/treq

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

Eliminate pyOpenSSL dependency #178

Merged
merged 3 commits into from Mar 27, 2017

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@markrwilliams @glyph
@markrwilliams
Member

markrwilliams commented Mar 27, 2017

As discussed in #175 (comment) and the following comments, treq should depend on a version of Twisted that itself depends on a Python 3 compatible version of pyOpenSSL. This allows treq to completely eliminate its pyOpenSSL dependency and with it tox environments that test incompatible combinations of pyOpenSSL and Twisted. These incompatible combinations prevent otherwise working PRs from building, like #177.

@glyph points out that these tox environments' intent might have been to exercise treq-Twisted-pyOpenSSL combinations that occur when a user relies on a system-provided Twisted. If that's the case, this PR might also need a documentation change that explains that installing from PyPI is the only officially supported means of acquiring treq.

markrwilliams added some commits Mar 26, 2017

@markrwilliams
Remove indepedent tls dependencies and their tox environments. 
Based on my comment here
#175 (comment):

I'm a little confused by the purpose of the
twisted_latest-pyopenssl_lowest environments. Twisted itself requires
a specific version of pyOpenSSL that's liable to be newer than the
lowest version treq claims to support. Indeed that's the case with
Twisted 17.1.0, which is why this environment fails:

https://travis-ci.org/twisted/treq/jobs/215313795

OP_SINGLE_ECDH_USE was introduced in pyOpenSSL 16.0.0, which Twisted
17.1.0 depends on. So it's no surprise that installing an older
version of pyOpenSSL will break things and it doesn't seem valuable to
test.

Why does treq need to depend on pyOpenSSL (or service_identity) at
all? Depending on Twisted with the tls extra installs the correct
versions of both. Removing a version specification for these
dependencies from treq's setup.py and the related tox environments
would make builds more robust and more honestly represent the actual
dependencies needed.
ef2ac7e
@markrwilliams
Regenerate .travis.yml
374e920

@markrwilliams markrwilliams referenced this pull request Mar 27, 2017

Closed

[WIP] Use a custom endpoint factory for testing. #175

@markrwilliams
Only supported Twisteds that depend on pyOpenSSL >= 16.0.0. 
Earlier Twisteds depend on a pyOpenSSL that doesn't support Python 3.

Exclude 17.1.0, because it introduced a regression:

https://twistedmatrix.com/trac/ticket/9032
Loading status checks…
fca95ce
@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov Mar 27, 2017

Codecov Report

Merging #178 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #178   +/-   ##
=======================================
  Coverage   96.55%   96.55%           
=======================================
  Files          20       20           
  Lines        1740     1740           
  Branches      154      154           
=======================================
  Hits         1680     1680           
  Misses         39       39           
  Partials       21       21

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e1df01f...fca95ce. Read the comment docs.

codecov commented Mar 27, 2017 •
edited

Codecov Report

Merging #178 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #178   +/-   ##
=======================================
  Coverage   96.55%   96.55%           
=======================================
  Files          20       20           
  Lines        1740     1740           
  Branches      154      154           
=======================================
  Hits         1680     1680           
  Misses         39       39           
  Partials       21       21

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e1df01f...fca95ce. Read the comment docs.
@glyph

This comment has been minimized.

Show comment
Hide comment
@glyph

glyph Mar 27, 2017

Member

Since I'm only guessing at the motivation (although to be fair, perhaps even if I were sure) I don't think we should block this on solving the decade-old problem of python user packaging ecosystem education. This looks great, thank you!
Member

glyph commented Mar 27, 2017

Since I'm only guessing at the motivation (although to be fair, perhaps even if I were sure) I don't think we should block this on solving the decade-old problem of python user packaging ecosystem education. This looks great, thank you!

@glyph glyph merged commit 47453b1 into twisted:master Mar 27, 2017
3 checks passed

3 checks passed

codecov/patch Coverage not affected when comparing e1df01f...fca95ce
Details
codecov/project 96.55% remains the same compared to e1df01f
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

twm added a commit to twm/treq that referenced this pull request Mar 30, 2017

@twm
Remove mention of dep versions from the docs 
As #178 shows, those changing setup.py cannot be relied upon to update
the documentation in parallel. Lacking an easy way to automate this,
I think it's best to just delegate communication of this this
information to PyPI.
ad5972f

@twm twm referenced this pull request Mar 30, 2017

Merged

Remove mention of dep versions from the docs #184

twm added a commit to twm/treq that referenced this pull request Mar 30, 2017

@twm
Remove mention of dep versions from the docs 
As #178 shows, it's easy to miss updating the docs when changing
setup.py.  Lacking an easy way to automate this, I think it's best to
just delegate communication of this this information to PyPI.
f8697c6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment