Improve error message if METADATA or PKG-INFO metadata is None #6542
Conversation
cjerdonek
force-pushed
the
issue-5082-missing-metadata-error
branch
from
db4906f
to
cd5bd2c
Compare
| # Use `dist` in the error message because its stringification | ||
| # includes more information, like the version and location. | ||
| return ( | ||
| 'None {} metadata found for distribution: {}'.format( |
There was a problem hiding this comment.
"No metadata found for distribution: {dist}" seems enough to me.
The distribution usually contains egg-info or dist-info extension on the string representation, right? If not, I'd prefer to keep the file name after the distribution, since that's more of a debugging detail IMO.
There was a problem hiding this comment.
"No metadata found for distribution: {dist}" seems enough to me.
There's a distinction here the message is trying to preserve. dist.has_metadata() can return False, which is one form of "no metadata," but the case of this exception is dist.has_metadata() True but the return value being None. The former case doesn't currently raise an exception and is being logged as, "No metadata found in ...". The latter is a very specific kind of error (and maybe shouldn't ever be happening).
The distribution usually contains egg-info or dist-info extension on the string representation, right?
I checked, and no, it doesn't include that.
There was a problem hiding this comment.
Thanks for taking a look. Your rationale sounds good to me.
There was a problem hiding this comment.
Okay, great. And thanks a lot for reviewing this quickly.
There was a problem hiding this comment.
No worries. Thanks for filing such polished PRs.
There was a problem hiding this comment.
By the way, the exception approach you liked was I think inspired subconsciously by this issue you had filed earlier: #6541 👍
lock
bot
added
the
auto-locked
Fixes #5082.