Skip to content

Make WHEEL file errors more explicit #7529

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

Merged
merged 13 commits into from
Dec 31, 2019

Conversation

chrahunt
Copy link
Member

@chrahunt chrahunt commented Dec 30, 2019

Removes an except Exception that was glossing over what could actually go wrong during parsing and extraction of Wheel-Version from the WHEEL metadata file. We should be able to transition this code much more easily to direct parsing of WHEEL (instead of using pkg_resources) and re-purpose it for reading from a zipfile directly.

The only change in behavior is an improvement in our error messages.

We plan to replace this code with direct extraction from a zip, so no
point catching anything more precise.
get_metadata delegates to the underlying implementation which tries
to locate and read the file, throwing an IOError (Python 2) or OSError
subclass on any errors.

Since the new explicit test checks the same case as brokenwheel in
test_wheel_version we remove the redundant test.

get_metadata may also throw a UnicodeDecodeError which we'll handle
next, so to preserve the existing behavior we return None on any other
exception.
This was the last error that could be thrown by get_metadata, so we can
also remove the catch-all except block.
This API does not raise an exception, but returns any errors on the
message object itself. We are preserving the original behavior, and can
decide later whether to start warning or raising our own exception.
`email.message.Message.__getitem__` returns None on missing values, so
we have to check for ourselves explicitly.
This is also the last exception that can be raised, so we remove
`except Exception`.
Since wheel_version never returns None, this exception will never be
raised.
@chrahunt chrahunt added type: refactor Refactoring code skip news Does not need a NEWS file entry (eg: trivial changes) labels Dec 30, 2019
@chrahunt chrahunt changed the title Refactor/cleanup wheel parsing Make WHEEL file errors more explicit Dec 30, 2019
@chrahunt chrahunt marked this pull request as ready for review December 31, 2019 00:18
@pradyunsg
Copy link
Member

(noting that I didn't run the code from this PR, to validate the error messages)

@chrahunt chrahunt merged commit 6914384 into pypa:master Dec 31, 2019
@chrahunt chrahunt deleted the refactor/cleanup-wheel-parsing branch December 31, 2019 15:17
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jan 30, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Jan 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation skip news Does not need a NEWS file entry (eg: trivial changes) type: refactor Refactoring code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants