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

Dependency cleanup #155

Merged
merged 4 commits into from Dec 8, 2016
Merged

Dependency cleanup #155

merged 4 commits into from Dec 8, 2016

Conversation

twm
Copy link
Contributor

@twm twm commented Dec 8, 2016

  • Remove setup.cfg [metadata] section, which didn't match setup.py and is deprecated anyway.
  • Instead of depending on Twisted's various TLS dependencies directly, depend on Twisted[tls] which has been present since at least Twisted 16.0.0 (but retain a higher PyOpenSSL dep for Python 3, since Twisted[tls] requires a lesser version).

* Remove setup.cfg [metadata] section, which didn't match setup.py and
  is deprecated anyway[1].
* Instead of depending on Twisted's various TLS dependencies directly,
  depend on Twisted[tls] which has been present since at least Twisted
  16.0.0 (but retain a higher PyOpenSSL dep for Python 3, since
  Twisted[tls] requires a lesser version).

[1]: https://wheel.readthedocs.io/en/latest/#defining-conditional-dependencies
@glyph
Copy link
Member

glyph commented Dec 8, 2016

Hopefully CI will be happy, but I'm inclined to merge.

@tomprince
Copy link
Contributor

It looks like the virtualenv that travis defaults to has a setuptools that isn't new enough to correctly support the environment markers used.

@glyph
Copy link
Member

glyph commented Dec 8, 2016

It looks like the virtualenv that travis defaults to has a setuptools that isn't new enough to correctly support the environment markers used.

Wait... virtualenv is the component that reads these descriptions?

@markrwilliams
Copy link
Member

markrwilliams commented Dec 8, 2016 via email

@twm
Copy link
Contributor Author

twm commented Dec 8, 2016

Is it not a serious problem that old setuptools can't install the package? I mean, the Python packaging morass wouldn't be such a problem if we could count on users to have the latest pip installed, would it?

Could we just require pyOpenSSL >= 0.15.1 in all cases? 0.13 is from 2011. The only reason I see keep using it (based on the changelog) is that it is the last release prior to cryptography and the associated cffi deps. Still, treq is unabashedly oriented towards communication on the Internet, and one should not communicate on the Internet with software from 2011...

@codecov-io
Copy link

codecov-io commented Dec 8, 2016

Current coverage is 96.31% (diff: 100%)

Merging #155 into master will not change coverage

@@             master       #155   diff @@
==========================================
  Files            19         19          
  Lines          1710       1710          
  Methods           0          0          
  Messages          0          0          
  Branches        152        152          
==========================================
  Hits           1647       1647          
  Misses           42         42          
  Partials         21         21          

Powered by Codecov. Last update 768c3f6...6dad622

@tomprince
Copy link
Contributor

Requiring a newer version seems reasonable (twisted 16.4 is requiring pyopenssl 16.0.0 anyway).

Regarding newer pip/setuptools, I know the most recent version of pip was the most common downloader on pypi a day after it was released.

@twm
Copy link
Contributor Author

twm commented Dec 8, 2016

service_identity has also dropped support for pyOpenSSL < 0.14, so I think that bumping treq's dependency is definitely the right move. I'm working on shepherding Travis to success now, which is proving a little difficult with the Fastly TLS incident.

@twm
Copy link
Contributor Author

twm commented Dec 8, 2016

@glyph If you still wish to merge, we're green here.

@glyph glyph merged commit f6f07be into twisted:master Dec 8, 2016
@twm twm deleted the dep-cleanup branch December 8, 2016 04:50
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

5 participants