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

Don't render all releases on version specific JSON pages #11775

Merged
merged 4 commits into from
Jul 7, 2022

Conversation

dstufft
Copy link
Member

@dstufft dstufft commented Jul 7, 2022

  • Reverts the streaming JSON, it didn't work and complicates the code/tests
  • Version specific legacy JSON pages only have information for that version now.
  • Non version specific legacy JSON pages still have information on all releases.

This is a breaking change, but it's a needed one if we're going to keep PyPI stable.

@dstufft dstufft requested a review from a team as a code owner July 7, 2022 14:31
Copy link
Member

@ewdurbin ewdurbin left a comment

Choose a reason for hiding this comment

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

probably want to update https://github.com/pypi/warehouse/blob/main/docs/api-reference/json.rst with the new form and a note on the breaking change?

otherwise, yes. its time.

@dstufft dstufft merged commit de96b45 into pypi:main Jul 7, 2022
@dstufft dstufft deleted the limit-json branch July 7, 2022 14:56
mnonnenmacher added a commit to oss-review-toolkit/ort that referenced this pull request Jul 8, 2022
The PyPI JSON API was changed to only include the URLs for the current
version on the version specific JSON pages [1].

[1]: pypi/warehouse#11775

Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.io>
mnonnenmacher added a commit to oss-review-toolkit/ort that referenced this pull request Jul 8, 2022
The PyPI JSON API was changed to only include the URLs for the current
version on the version specific JSON pages [1].

[1]: pypi/warehouse#11775

Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.io>
@EcmaXp
Copy link

EcmaXp commented Jul 8, 2022

With this change, I think the poetry lock process is now failing: python-poetry/poetry#5970

I checked the metrics in PyPI Public Dashboard, and it makes sense that this change had to be merged quickly.

However, I still think that there is a way to revert the previous PR or rollback to the previous version rather than making a breaking change in the API.

It would be nice to have a way to notify the package manager maintainer if there is a case where API breaking changes are made in the future.

Thanks for your contribution. I'm glad that PyPI is stable for now. 🎉

@mkniewallner
Copy link

If the change was to be reverted, on Poetry side we would probably only need to bring back the version requested in /pypi/<project_name>/<version>/json endpoint in releases key, because that's exactly what we did before this change.

So for instance, if we query /pypi/requests/2.28.1/json, we would only need 2.28.1 in releases, not the other versions.

I understand that a revert (even a partial one as suggested that would add back only the requested version) is cumbersome and a pretty big request (especially since this PR is about PyPI stability, which is obviously important), but just to bring awareness about the impact, basically all released versions of Poetry (as of writing this) are affected, preventing the retrieval of hashes when locking dependencies, since they rely on releases key, so the impact is quite high for Poetry end users.

We are in the process of releasing an express new Poetry 1.1 version that relies on urls key instead of releases, so people will have a workaround, but people with older versions will have no other choice than updating, which can be quite disruptive.

@dstufft
Copy link
Member Author

dstufft commented Jul 8, 2022

I went back and forth on whether to leave the releases key there with just the current version in it or not.

I ultimately decided not to for one major, and one minor reason.

The minor reason is just that the data already exists on the response in the urls key, so it was duplicative.

The major reason though is that I think breaking loudly is better than breaking quietly, and while poetry's specific use here wouldn't be affected, it's just as easy to imagine someone iterating over the list of releases rather than looking for a specific release. If they did that, then they would get silently wrong data instead of breaking loudly.

@mkniewallner
Copy link

The major reason though is that I think breaking loudly is better than breaking quietly, and while poetry's specific use here wouldn't be affected, it's just as easy to imagine someone iterating over the list of releases rather than looking for a specific release. If they did that, then they would get silently wrong data instead of breaking loudly.

This is perfectly understandable. I've only recently started contributing more and more to Poetry (so disclaimer, not a core contributor), so I lack a bit of context to know if it should/could have used urls from the beginning, but on a broader level, it's a bit frightening that Poetry relies on an API that could break (even if the reasons to break things are perfectly valid, so I'm mostly reasoning from a Poetry end user and maintainer point of view).

Was there a way to know beforehand about the removal of releases in the API? Maybe in a mailing list for instance?

I've noticed #11559 following another issue that recently affected Poetry, and I agree that it would be awesome to document a bit more what to expect from the different APIs in terms of stability. Having deprecation notices, even if short ones, would at least make it possible to update the API requests on Poetry side, which would be a better experience for Poetry users since it would give them time to upgrade rather than being forced to.

@richaagarwal
Copy link

Hi there - thanks for the context on this change. I'm curious if/when the releases key will be removed on the non-versioned json API endpoint as well. I noticed that the documentation was updated to include a note that it may be deprecated in the future; any timeline you are able to share with me would be helpful!

@dstufft
Copy link
Member Author

dstufft commented Jul 13, 2022

As far as we can tell, there's not currently any major problems stemming from that specific key since it's one extra response to cache (versus an unbounded number of extra responses to cache when it was on the versioned URL). Given that, there's currently not any specific plan to remove the releases key from the non-versioned URL, so the answer to that is basically "nobody knows, it might never be removed, or it might be in the future if it starts to cause problems because it makes the responses huge".

That warning was put there mostly to guide people to attempt to find an alternative solution if one exists to relying on that key OR if an alternative solution doesn't exist, to follow up on the issue tracker or on discuss.python.org with their use cases that aren't handled currently by the other keys in the JSON API or, even better, by the simple repository API so we can figure out if there's a better, less problematic, answer we can add to support them.

DMRobertson pushed a commit to matrix-org/synapse that referenced this pull request Jul 15, 2022
[1.1.14](https://github.com/python-poetry/poetry/releases/tag/1.1.14)
includes a critical bugfix needed to generate lockfiles when using PyPI
as the source of packages.
(python-poetry/poetry#5973, to accomodate
pypi/warehouse#11775)

[1.1.13](https://github.com/python-poetry/poetry/releases/tag/1.1.13)
includes bugfixes that shouldn't be relevant for us (`poetry self
update` and Windows fixes only).

We shouldn't need to use 1.1.14 in the Docker image as we never `lock`
nor `update`, but I have included it for consistency.

I've also added a quick troubleshooting guide to the dev docs.
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Nov 10, 2022
…sitory hash issue r=firefox-build-system-reviewers,glandium

There is a bug in Poetry 1.2.0a2 caused by a breaking change
on PyPi's end: pypi/warehouse#11775
This bug was fixed in Poetry 1.2.0b3, but 1.2.0b1 dropped
support for Python 3.6, which is a requirement for us.
As a temporary workaround, we can patch the fix into our
virtualenv's copy of Poetry. This patch should be removed
once we switch to using a newer version of Poetry.

Differential Revision: https://phabricator.services.mozilla.com/D161544
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Nov 11, 2022
…sitory hash issue r=firefox-build-system-reviewers,glandium

There is a bug in Poetry 1.2.0a2 caused by a breaking change
on PyPi's end: pypi/warehouse#11775
This bug was fixed in Poetry 1.2.0b3, but 1.2.0b1 dropped
support for Python 3.6, which is a requirement for us.
As a temporary workaround, we can patch the fix into our
virtualenv's copy of Poetry. This patch should be removed
once we switch to using a newer version of Poetry.

Differential Revision: https://phabricator.services.mozilla.com/D161544
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants