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

On Travis, follow package dependency graph to expand package list. #2487

Closed
wants to merge 2 commits into from

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Oct 4, 2016

To make computing the dependency graph easier I moved the requirements of each package into a requirements.txt file.

I'm happy to instead write a custom setup.py parser but decided that the route of less code was "better" in some sense.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 4, 2016
@daspecster
Copy link
Contributor

@dhermes can you fix the conflicts?
Otherwise this LGTM.

Using boilerplate code snippet in each setup.py to load
the requirements.txt and adding the requirements.txt file to
MANIFEST.in file.
@dhermes dhermes force-pushed the compute-dep-graph branch from 31d2f51 to f2b87af Compare October 4, 2016 17:25
Copy link
Contributor

@tseaver tseaver left a comment

Choose a reason for hiding this comment

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

I am -1 on the "parse requirements.txt" approach:

  • The requirements.txt format is effectively an internal implementation detail of pip: they provide no public API for parsing it.
  • In the words of the PyPA docs and @dstufft, it is intended to support "application deployments", not as a substitute driver for install_requires / "library requirements."

I believe we would be better off leaving install_requires as the definitive version spec for the library, and using the pkg_resources API in our scripts to get the dependency metadata we need.

/cc @jonparrott

@dhermes
Copy link
Contributor Author

dhermes commented Oct 4, 2016

@tseaver SGTM. @jonparrott and I discussed and I view pkg_resources as inferior to parsing because it requires having those packages installed somewhere before we decide we want to run those tests.

I'll whip up a proof of concept that parses our setup.py files, assuming some kind of format, and enforces the assumptions with errors.

@tseaver
Copy link
Contributor

tseaver commented Oct 4, 2016

@dhermes To do the analysis on Travis, we are going to have them installed somewhere, likely into a tox environment. If we add a console_script (maybe as an extra) which is installed into the environment, then we can get the metadata from it.

@dhermes
Copy link
Contributor Author

dhermes commented Oct 4, 2016

@tseaver PTAL at #2498

@dhermes dhermes closed this Oct 4, 2016
@dhermes dhermes deleted the compute-dep-graph branch October 4, 2016 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement. testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants