Skip to content

Comments

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

Merged
finswimmer merged 3 commits intopython-poetry:masterfrom
mkniewallner:fix/fix-releases-key-pypi-repository
Jul 8, 2022
Merged

fix(repositories): use urls for versioned JSON API instead of releases#5972
finswimmer merged 3 commits intopython-poetry:masterfrom
mkniewallner:fix/fix-releases-key-pypi-repository

Conversation

@mkniewallner
Copy link
Member

@mkniewallner mkniewallner commented Jul 8, 2022 •
edited
Loading

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 mkniewallner mentioned this pull request Jul 8, 2022
Closed
3 tasks
@mkniewallner
Copy link
Member Author

mkniewallner commented Jul 8, 2022 •
edited
Loading

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 •
edited
Loading

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

mkniewallner commented Jul 8, 2022

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

Agree, though releases is also used in

poetry/src/poetry/repositories/pypi_repository.py

Line 66 in 584aaee

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

mkniewallner commented Jul 8, 2022

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 •
edited
Loading

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
finswimmer
finswimmer approved these changes Jul 8, 2022
View reviewed changes
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 mkniewallner mentioned this pull request Jul 8, 2022
Merged
2 tasks
@finswimmer finswimmer merged commit bf885ac into python-poetry:master Jul 8, 2022
@mkniewallner
Copy link
Member Author

mkniewallner commented Jul 8, 2022

Can you please prepare a backport to 1.1?

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

@mkniewallner mkniewallner deleted the fix/fix-releases-key-pypi-repository branch July 8, 2022 12:21
This was referenced Jul 8, 2022
Closed
Closed
@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

mkniewallner commented Jul 11, 2022

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

dwangfaulkner-fn commented Jul 11, 2022

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
Merged
@mkniewallner
Copy link
Member Author

mkniewallner commented Jul 14, 2022

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.

@agbrooks agbrooks mentioned this pull request Aug 5, 2022
Merged
1 task
@github-actions
Copy link

github-actions bot commented Feb 29, 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 Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@finswimmer finswimmer finswimmer approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Breaking change in Warehouse breaks poetry lock file creation Poetry not passing through SHAs for certain package versions.

5 participants

@mkniewallner @finswimmer @dimbleby @mbelang @dwangfaulkner-fn