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

Fix ValueError when finding packages with invalid metadata #1346

Merged
merged 1 commit into from
Sep 13, 2019

Conversation

sztomi
Copy link
Contributor

@sztomi sztomi commented Sep 1, 2019

Some packages (like pygame-music-grid) have unparseable metadata. This resulted in a ValueError. pip ignores packages like these, so it's reasonable to do the same in poetry itself and only display the well-formed ones.

Fixes #1345.

Pull Request Check List

  • Added tests for changed code.
  • Updated documentation for changed code.
    • n/a: this is a bugfix

@brycedrennan brycedrennan added the kind/bug Something isn't working as expected label Sep 1, 2019
Copy link
Contributor

@brycedrennan brycedrennan left a comment

Choose a reason for hiding this comment

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

Thanks for tackling this.

A couple things:

  • It looks like a test could go in test_pypi_repository.py. You could mock out ServerProxy to return test data with invalid versions.
  • If possible, I'd like to more narrowly catch the error. With a testcase it will be easier to see if this is possible

Some packages (like `pygame-music-grid`) have unparseable metadata.
This resulted in a ValueError. pip ignores packages like these, so it's reasonable
to do the same in poetry itself and only display the well-formed ones.

Fixes python-poetry#1345.
Fixes python-poetry#637.
@sztomi
Copy link
Contributor Author

sztomi commented Sep 8, 2019

Hi @brycedrennan, I updated my branch with:

  • A more strict exception (ParseVersionError) that is actually raised instead of ValueError
  • A unit test for the issue along with a malformed json package metadata that would cause an issue without the fix. I actually discovered that a very similar logic is implemented in the find_packages method so I added that to the same test.

'Unable to parse version "{}" for the {} package, skipping'.format(
hit["version"], hit["name"]
),
level="debug",
Copy link
Contributor

@bjoernpollex bjoernpollex Sep 9, 2019

Choose a reason for hiding this comment

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

I wonder if this should be a warning that's printed to the shell? This way it's much more likely that someone will raise the issue with the upstream.

Copy link
Contributor Author

@sztomi sztomi Sep 9, 2019

Choose a reason for hiding this comment

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

I think there is a fairly large number of malformed (and abandoned) packages on pypi that are easy to run into when you search for a fragment of a package name. This is almost the same logging that is done in find_packages in the same file.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point - and it would still show up when running with higher verbosity, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep

@sdispater sdispater merged commit 58ba79c into python-poetry:master Sep 13, 2019
@sdispater
Copy link
Member

Thanks!

@sztomi sztomi deleted the fix_pygame_search branch September 13, 2019 15:57
@randallpittman
Copy link

randallpittman commented Nov 30, 2020

N.B. This does not fix having already-installed packages with un-parseable versions. It seems to only fix Poetry searches that result in unparseable versions. See #2167.

Copy link

github-actions bot commented Mar 1, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Something isn't working as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Searching for pygame results in ValueError
5 participants