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

Avoid the deprecated JSON API #6081

Merged
merged 1 commit into from
Sep 17, 2022

Conversation

dimbleby
Copy link
Contributor

Resolves #6076

I've taken the JSON version of the simple API and converted it into a LinkSource so that the package-finding logic in the PyPiRepository is very similar to - but annoyingly not quite the same as! - the LegacyRepository.

I've also taken the opportunity to refactor the LegacyRepository ever so slightly to emphasise that similarity. I think I've probably fixed a small bug re caching and pre-releases: previously the processing for ignored pre-releases was skipped when reading from the cache.

I believe this change will tend to be a modest performance hit. Eg consider a package like cryptography, for which there are maybe a couple of dozen downloads available at each release: to get the available versions we now have to iterate over each of those files and parse their names, rather than simply reading the answer.

However if the API that poetry currently uses is truly deprecated I see little choice but to suck that up - or risk being in an awkward spot when it is turned off. cf #5970, but worse.

Most of the changes are in the test fixtures:

  • unversioned fixtures were generated from the existing fixtures: I didn't want to download fresh data and start getting different answers than the tests were expecting
  • new versioned fixtures were downloaded fresh

@neersighted
Copy link
Member

I think, given the state of discourse on DPO, this is a direction we should move in. I haven't had time to look at the specific implementation (and we need to think about it in the context of #5442, which still needs some adjustment as I recall), but I'd like to make sure we get this done for 1.3.

@neersighted neersighted added this to the 1.3 milestone Sep 5, 2022
@dimbleby
Copy link
Contributor Author

dimbleby commented Sep 5, 2022

I'm actually optimistic that they'll add versions before removing releases - I have pypi/warehouse#12079 open and I see that thread talks about similar.

So it may be that this is not needed.

But it would be good to have lines of communication open: the sudden and unexpected removal of API features that poetry uses is certainly undesirable...

@neersighted
Copy link
Member

neersighted commented Sep 5, 2022

Ultimately I'd like to get at least one warehouse contributor involved in the project (could be being available to talk shop on Discord, monitoring these issues/interested in pings, or even becoming a regular contributor), which would likely head off a lot of these issues. Of course one of the simplest ways to do that would be one of the regulars here contributing to Warehouse itself 😆

Thanks for that PR link -- I was unaware of it.

Regardless of what happens with the JSON API, I do think a move back towards the PEP 503 API is the way to go, as in the long term PEP 658 and 691 are meant to replace the current JSON API for everything we need from PyPI, and this architecture will make that transition easier. Of course we could do that later in one fell swoop, but I don't see the harm with your take from a quick skim.

@dimbleby
Copy link
Contributor Author

dimbleby commented Sep 5, 2022

this MR has a relatively painful-looking conflict with the handling of yanked releases

@neersighted
Copy link
Member

neersighted commented Sep 5, 2022

Certainly wouldn't fault you for dropping it, as it's incremental progress, but ultimately not a stepping stone we can't do without.

@neersighted neersighted added area/solver Related to the dependency resolver kind/refactor Pulls that refactor, or clean-up code impact/backport Requires backport to stable branch impact/changelog Requires a changelog entry backport/1.2 area/sources Releated to package sources/indexes/repositories and removed area/solver Related to the dependency resolver labels Sep 17, 2022
@neersighted neersighted enabled auto-merge (squash) September 17, 2022 17:31
@neersighted neersighted merged commit b61a4dd into python-poetry:master Sep 17, 2022
poetry-bot bot pushed a commit that referenced this pull request Sep 17, 2022
Resolves #6076

I've taken the JSON version of the simple API and converted it into a
`LinkSource` so that the package-finding logic in the `PyPiRepository`
is very similar to - but annoyingly not quite the same as! - the
`LegacyRepository`.

I've also taken the opportunity to refactor the `LegacyRepository` ever
so slightly to emphasise that similarity. I think I've probably fixed a
small bug re caching and pre-releases: previously the processing for
ignored pre-releases was skipped when reading from the cache.

I believe this change will tend to be a modest performance hit. Eg
consider a package like `cryptography`, for which there are maybe a
couple of dozen downloads available at each release: to get the
available versions we now have to iterate over each of those files and
parse their names, rather than simply reading the answer.

However if the API that poetry currently uses is truly deprecated I see
little choice but to suck that up - or risk being in an awkward spot
when it is turned off. cf #5970, but worse.

Most of the changes are in the test fixtures:
- unversioned fixtures were generated from the existing fixtures: I
didn't want to download fresh data and start getting different answers
than the tests were expecting
- new versioned fixtures were downloaded fresh

(cherry picked from commit b61a4dd)
@dimbleby dimbleby deleted the avoid-deprecated-api branch September 17, 2022 17:33
neersighted pushed a commit that referenced this pull request Sep 17, 2022
Resolves #6076

I've taken the JSON version of the simple API and converted it into a
`LinkSource` so that the package-finding logic in the `PyPiRepository`
is very similar to - but annoyingly not quite the same as! - the
`LegacyRepository`.

I've also taken the opportunity to refactor the `LegacyRepository` ever
so slightly to emphasise that similarity. I think I've probably fixed a
small bug re caching and pre-releases: previously the processing for
ignored pre-releases was skipped when reading from the cache.

I believe this change will tend to be a modest performance hit. Eg
consider a package like `cryptography`, for which there are maybe a
couple of dozen downloads available at each release: to get the
available versions we now have to iterate over each of those files and
parse their names, rather than simply reading the answer.

However if the API that poetry currently uses is truly deprecated I see
little choice but to suck that up - or risk being in an awkward spot
when it is turned off. cf #5970, but worse.

Most of the changes are in the test fixtures:
- unversioned fixtures were generated from the existing fixtures: I
didn't want to download fresh data and start getting different answers
than the tests were expecting
- new versioned fixtures were downloaded fresh

(cherry picked from commit b61a4dd)
@radoering
Copy link
Member

I believe this change will tend to be a modest performance hit. Eg consider a package like cryptography, for which there are maybe a couple of dozen downloads available at each release: to get the available versions we now have to iterate over each of those files and parse their names, rather than simply reading the answer.

Some measurements would be interesting.

@dimbleby Since you already executed that one performance test in the other issue, you may want to do it for this change, too?

@radoering
Copy link
Member

For the record: There seems to be no measurable performance regression when running the example from the shootout.

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
area/sources Releated to package sources/indexes/repositories impact/backport Requires backport to stable branch impact/changelog Requires a changelog entry kind/refactor Pulls that refactor, or clean-up code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The "releases" key on the pypi JSON API is deprecated
3 participants