-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Extended PEP 381 mirror and package verification features. #402
Conversation
This includes the following modifications: - The PyPI mirrors are now used by default. You can disable them by specifying `--no-mirrors`. Each mirror is now tried one by when when retrieving a distribution instead of naively randomizing the list of index URLs. Distributions from mirrors whose signature don't match are ignored. - Added `--refresh-serverkey` option to `install` command to retrieve the public PyPI server key used to verify the mirror packages, caching it as ~/.pip/serverkey.pub. Same thing happens (once) it doesn't exist. - Added ability to verify the SSL certificate of PyPI. This requires the ssl module added in Python 2.6. For Python < 2.6 the standalone version at http://pypi.python.org/pypi/ssl can be installed instead. Using https://pypi.python.org/ is the default now. - Added OrderedDict for Python < 3.1 used in PackageFinder when collecting packages links for easier handling. - A bunch of PEP 8 related fixes.
@@ -412,14 +469,13 @@ def unpack_http_url(link, location, download_cache, only_download): | |||
target_file = None | |||
download_hash = None | |||
if download_cache: | |||
target_file = os.path.join(download_cache, | |||
urllib.quote(target_url, '')) | |||
cache_filename = list(filter(None, target_url.split('/')))[-1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the purpose of the filter call here?
Also, I'm not sure I agree with this change in behavior - it seems like this could make it easier to get a polluted download cache if you once get a wrong version of a tarball with a given name, from some bad location, it would then mask getting it from the correct location later. But I have a vague memory we discussed this, so maybe there's good reason for it and I'm just forgetting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The filter call removes empty items of a list, e.g.:
>>> p = [1, 2, 3, '', 4, 5]
>>> filter(None, p)
[1, 2, 3, 4, 5]
That's to make sure double slashes don't create wrong cache filenames.
Regarding the changed cache behaviour, yeah, I agree this might lead to half downloaded cached files, but since we also check for the MD5 of those files it should be fine. Not sure what we could do here better, with the mirror system in place we have to make sure that not each mirror creates a separate cached file (due to the URL being encoded in the old cache filename).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yeah, I remember our conversation now. Yes, with mirrors the current cache behavior isn't really workable. I guess it does make sense that MD5 checking ought to take care of any issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use filter(bool, thingy)
instead, which makes a bit more sense IMO.
I wonder if it would be doable to pull the variable naming, whitespace, and other PEP8 fixes out of this branch and apply them to develop separately. That would make the diff so much smaller and easier to review the actual changes in behavior - I'm finding it quite difficult to tease out where the behavior is actually being changed in the package finder. |
I could work on that over the next days, yeah. I think that was a really bad idea, d'oh. |
This is exciting. Hope it gets merged in soon. :) |
Conflicts: pip/index.py pip/locations.py pip/req.py
serverkey_cache.write(content) | ||
except Exception: | ||
e = sys.exc_info()[1] | ||
raise |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo here? There's an empty raise and then a raise with an explicit exception.
Conflicts: pip/basecommand.py pip/download.py
Is this stalled still? |
Closing this pull request - there's a new branch in progress for this work. |
This includes the following modifications:
by specifying
--no-mirrors
. Each mirror is now tried one bywhen when retrieving a distribution instead of naively randomizing
the list of index URLs. Distributions from mirrors whose signature
don't match are ignored.
--refresh-serverkey
option toinstall
command to retrievethe public PyPI server key used to verify the mirror packages,
caching it as ~/.pip/serverkey.pub. Same thing happens (once) it
doesn't exist.
the ssl module added in Python 2.6. For Python < 2.6 the standalone
version at http://pypi.python.org/pypi/ssl can be installed instead.
Using https://pypi.python.org/ is the default now.
collecting packages links for easier handling.