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(repositories): use urls for versioned JSON API instead of releases #5972

Merged
merged 3 commits into from
Jul 8, 2022
Merged

fix(repositories): use urls for versioned JSON API instead of releases #5972

merged 3 commits into from
Jul 8, 2022

Conversation

mkniewallner
Copy link
Member

@mkniewallner mkniewallner commented Jul 8, 2022

Pull Request Check List

Resolves: #5967
Resolves: #5970

  • Added tests for changed code.
  • Updated documentation for changed code. Not applicable

When retrieving package metadata for a specific version from pypi.org JSON API, Poetry currently relies on a releases keys to look for the correct version based on the URL.
Following pypi/warehouse#11775, releases has been removed from /pypi/<package_name>/<version>/json endpoints, so Poetry can't pick up this metadata anymore.

There's no real reason to use releases, since urls is also available, and has the advantage of directly returning the version we are looking for (so instead of checking for data["releases"][version], we directly access the version we are looking for in data["urls"]).

Note: Given that this makes Poetry locking mechanism broken on all released versions (not only on master), this would probably require a backport to the 1.1 branch, even if it is mostly unmainted. Started
https://github.com/mkniewallner/poetry/commits/fix/fix-releases-key-pypi-repository-1.1 for that.

@mkniewallner
Copy link
Member Author

mkniewallner commented Jul 8, 2022

Tests pass (minus the flaky 3.11-dev on macOS), which means we definitely lack some test coverage for this.

@finswimmer
Copy link
Member

finswimmer commented Jul 8, 2022

Tests pass (minus the flaky 3.11-dev on macOS), which means we definitely lack some test coverage for this.

I think the test_package test in test_pypi_repository.py is a good candidate for extending the test. We have to check for the content of the files attribute.

Probably the json files under tests/repositories/fixtures/pypi.org/json must be adopted.

@mkniewallner
Copy link
Member Author

Probably the json files under tests/repositories/fixtures/pypi.org/json must be adopted.

Agree, though releases is also used in

for version, release in info["releases"].items():

So we will probably need to change that too.

@dimbleby
Copy link
Contributor

dimbleby commented Jul 8, 2022

will probably need to change that too.

I don't think so - I think that is looking at the reponse from the non-versioned page, which continues to include this information

@mkniewallner
Copy link
Member Author

I don't think so - I think that is looking at the reponse from the non-versioned page, which continues to include this information

Oh yeah, https://github.com/pypi/warehouse/pull/11775/files#diff-beb4788f749d9d561005b6542d39b9f7d6f0ed2e7e9bd8b614673294aab9b488R188-R189 you're right indeed, /pypi/<project_name>/json is untouched, my bad for overlooking that.

@mkniewallner
Copy link
Member Author

mkniewallner commented Jul 8, 2022

Updated the PR to:

  • add a dedicated version fixture for requests without a releases key in 9f29869 (so it uses this file instead of the global one, according to this code)
  • remove all releases key from the versioned fixtures in 6b40ebc

@mkniewallner mkniewallner marked this pull request as ready for review July 8, 2022 11:48
@mkniewallner mkniewallner requested a review from a team July 8, 2022 12:07
@mkniewallner mkniewallner changed the title fix(repositories): use urls for JSON API instead of releases fix(repositories): use urls for versioned JSON API instead of releases Jul 8, 2022
Copy link
Member

@finswimmer finswimmer left a comment

Choose a reason for hiding this comment

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

LGTM and also tested this locally 🏅 🚀

Can you please prepare a backport to 1.1?

@mkniewallner
Copy link
Member Author

Can you please prepare a backport to 1.1?

Yes, PR is #5973, just waiting for CI to pass.

@mbelang
Copy link

mbelang commented Jul 11, 2022

Updated to 1.1.14 and still doing it. Is there something to modify to actually fix the issue locally?

@mkniewallner
Copy link
Member Author

Updated to 1.1.14 and still doing it. Is there something to modify to actually fix the issue locally?

Yes, you have to clear Poetry's cache first:

poetry cache clear pypi --all

@mbelang
Copy link

mbelang commented Jul 11, 2022

Yeah it works thx. I found the answer in one of the issues.

@dwangfaulkner-fn
Copy link

Sorry to jump onto this thread but is there anywhere to see when a new beta release will be cut with this very critical fix? We'd prefer to use a released version instead of building from main, as I'm sure many other folks would.

@mkniewallner mkniewallner mentioned this pull request Jul 13, 2022
@mkniewallner
Copy link
Member Author

Sorry to jump onto this thread but is there anywhere to see when a new beta release will be cut with this very critical fix? We'd prefer to use a released version instead of building from main, as I'm sure many other folks would.

A new beta for 1.2 has been released: https://github.com/python-poetry/poetry/releases/tag/1.2.0b3.

Copy link

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 Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
5 participants