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

Use pypiorg instead #52

Merged
merged 3 commits into from
Mar 19, 2018
Merged

Use pypiorg instead #52

merged 3 commits into from
Mar 19, 2018

Conversation

peterbe
Copy link
Owner

@peterbe peterbe commented Mar 19, 2018

Seems to work in theory (and unit tests).

Changes:

  1. Instead of https://pypi.python.org/pypi/<package>/json it uses https://pypi.org/pypi/<package>/json instead.
  2. The new JSON API has a sha256 digest included already for every release. So if the chosen algorithm is sha256 we can just extract it from the JSON instead of having to download the file and run pip on it.
  3. Drop support for python 2.6 and 3.3 (because Travis can't install them it seems)

Copy link
Contributor

@di di left a comment

Choose a reason for hiding this comment

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

Hey, I saw you were looking for a review of this. As far as integrating w/ the new PyPI, this looks good to me!

@peterbe
Copy link
Owner Author

peterbe commented Mar 19, 2018

Thanks @di!
It still uses internal pip function if the hash algorithm you want isn't in the JSON output.

Perhaps something like this:

if args.algorithm != 'sha256' and args.algorithm not in digests:
    print("Your preferred algorithm isn't available in the JSON. It has to be computed for every file and every file needs to be downloaded. Are you sure you can't use 'sha256'?")

...or something like that.

@di
Copy link
Contributor

di commented Mar 19, 2018

@peterbe Makes sense. You might want to try to shell out to pip hash instead of using the internal API, since this will be changing in pip==10.0.0 (and might already be broken with pip==9.1.2)

@peterbe
Copy link
Owner Author

peterbe commented Mar 19, 2018

@di Yeah, I saw some scary noise about that in other pypi issues today. The difference between 9.0.1 and 9.0.2 broke for some people.
I'd love to switch to shell but it's not a small change. This PR is big enough as it is. Also, I worry about getting the right pip in the shell, or do you think that's easy to get right?

@di
Copy link
Contributor

di commented Mar 19, 2018

@peterbe Yeah, it would be a big change, but one you're probably going to have to make soon.

I'm not totally sure what you mean about getting the "right" pip -- I think you'd just get whatever was available in the context that hashin was run in.

@peterbe
Copy link
Owner Author

peterbe commented Mar 19, 2018

@di If you install hashin globally and it calls out to pip it will use the "system pip". Not necessarily an up-to-date one. E.g. if you let Ubuntu install pip you might get an ancient version and not the one you use in your relevant virtualenv.

Perhaps it doesn't matter. All we're asking this pip executable (in the shell) to do is generate a sha512 hash of a file on disk. Might not matter so much what version of pip it is and/or if it's a pip you didn't have in your current virtualenv.

@di
Copy link
Contributor

di commented Mar 19, 2018

@peterbe Could always invoke it with python -m pip hash instead, that would at least ensure that you're getting the same pip that you would get from import pip.

I have some ideas about making a pip-api project (I also maintain a project which (ab)uses pip in this way), I'll reach out to you when it comes to fruition.

@peterbe
Copy link
Owner Author

peterbe commented Mar 19, 2018

Do you mean to change this line to #!/usr/bin/env python -m pip?

@di
Copy link
Contributor

di commented Mar 19, 2018

Ah, no, I'm saying instead of shelling out to pip with something like:

subprocess.call(['pip', 'hash', ...])

you would do

subprocess.call(['python', '-m', 'pip', 'hash', ...])

@peterbe peterbe merged commit 4d73ae2 into master Mar 19, 2018
@peterbe peterbe deleted the use-pypiorg-instead branch March 19, 2018 23:35
@peterbe
Copy link
Owner Author

peterbe commented Mar 20, 2018

Blogged about it: https://www.peterbe.com/plog/hashin-0.12.0

@peterbe
Copy link
Owner Author

peterbe commented Mar 20, 2018

@di What version of Python does that require? I think hashin works in Python 2.6 even though it's no longer being tested in Travis.

@di
Copy link
Contributor

di commented Mar 20, 2018

@peterbe It was added in Python 2.5: https://www.python.org/dev/peps/pep-0338/

@peterbe
Copy link
Owner Author

peterbe commented Mar 20, 2018

@di Awesome! Let's do it #58

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.

2 participants