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

Implement 'latest' API and release-view endpoints #8615

Open
wants to merge 42 commits into
base: main
Choose a base branch
from

Conversation

bskinn
Copy link
Contributor

@bskinn bskinn commented Sep 28, 2020

Implements and closes #4663. See here for the semantics of the three new endpoints.

Per here and here, there was interest expressed in also having similar latest endpoints available for release views on the web UI. This PR has been expanded beyond #4663 to include all three of the analogous release-view redirects, at /project/{name}/latest, etc.

As the release calculations for the new latest JSON and web UI endpoints use the .latest_version property that was previously defined on Project, their implementations are coupled to the behavior used to serve the /project/{name}/ endpoint.

Conversely, the latest-stable and latest-unstable endpoints for both the JSON and web UI are based on new functionality, and are not coupled to any other behaviors.

  • Implementation
  • Tests
  • Docs
  • Changelog/Announcement (?) N/A

@di
Copy link
Member

di commented Sep 28, 2020

Nice! Just an FYI @bskinn, we'll hold off on giving this a full review until you take it out of draft status, but if you want input at any point feel free to @-mention me with specific questions.

@bskinn
Copy link
Contributor Author

bskinn commented Sep 28, 2020

Yep, @di, that was my intention with draft status... no need to take maintainer time/attention until I've got it fleshed out. 👍

@bskinn bskinn force-pushed the latest-json branch 4 times, most recently from c8420e6 to 0760048 Compare September 30, 2020 19:20
@bskinn
Copy link
Contributor Author

bskinn commented Oct 1, 2020

Ok, @di, I think this is ready for review on the implementation and tests. I'll augment the API docs as part of this PR, too--still TODO for now. (If you want to wait to review until I've finished the docs update, let me know.)

One big question: with the arc I took on the code, I ended up implementing the logic directly in warehouse/legacy/api/json.py, and not as methods on the Project model. Would you like me to refactor it over to such methods? I could see Project.latest_stable_version and Project.latest_unstable_version as potentially being useful elsewhere; or, they could just be YAGNI at this point.

@bskinn bskinn marked this pull request as ready for review October 1, 2020 01:04
@bskinn
Copy link
Contributor Author

bskinn commented Oct 1, 2020

Looks like there is existing interest in having access to behaviors that would use latest-unstable (at least) in addition to latest. I'm inclined to refactor the lookup into Project.

badges/shields#4692

#4663 (comment)

@bskinn bskinn changed the title Implement 'latest' API endpoints Implement 'latest' API and release-view endpoints Oct 2, 2020
@bskinn bskinn marked this pull request as draft October 2, 2020 21:06
@bskinn bskinn marked this pull request as ready for review October 4, 2020 15:56
@bskinn
Copy link
Contributor Author

bskinn commented Oct 4, 2020

Ok, @di, this is ready for review. Code/tests/docs are all completed for both the JSON and web UI latest endpoints.

I couldn't find anywhere to put a news snippet (as per, e.g., towncrier), or to include a CHANGELOG entry. Please advise if that's something I need to do, and how.

Base automatically changed from master to main January 21, 2021 18:39
@bskinn
Copy link
Contributor Author

bskinn commented Jan 22, 2021

Hi, @di -- I'm still interested in shepherding this PR through, if the features remain of interest. Please let me know if there's anything I can do on my end to facilitate the process.

Need to add trailing-slash redirect, also.
Always want the browser to recheck this endpoint, in case the
redirect target changes.
Still need to actually test the latest logic.
Switch latest from a redirect to /pypi/{name}/json, and instead
duplicate the version-search logic. This should make `latest` robust
against any future changes to the `json_project` logic.

Implement latest-stable and latest-unstable logic.
Need to fix unstable query (or test?), is redirecting to the
most recent stable version instead of most recent prerelease
for project_with_pre
Apparently, the _pypi_ordering is a function of the order in which
Releases are added to a Project.

Seems potentially brittle.
@graingert
Copy link
Contributor

graingert commented Apr 28, 2021

From my GitHub readme, I'd want to link to latest pre-release unless there's a newer main release. But for searches I'd want people to always land on the latest main release

@ewjoachim
Copy link
Contributor

ewjoachim commented Apr 28, 2021

And on the other GitHub badge you mentioned that pointed to the latest stable, did you have in mind a page pointing on a specific release or the project page we have today? (Upon visiting the link, would you imagine the displayed install instructions be pip install foo or pip install foo==1.2.3)

(Edit: I guess it's the same for the "latest", would you expect pip install foo --pre or pip install foo==1.2.3b17)

JSON API views now directly return, rather than redirecting.

Per
pypi#8615 (review)
For completeness, and to satisfy 100% coverage requirement.
Addresses pypi#8615 (comment).

The database queries inline here may be duplicative; will check
in a subsequent commit.
The 'latest' members off of the Project model now return
fully-realized Release instances, making a subsequent database
call unnecessary.
@bskinn
Copy link
Contributor Author

bskinn commented May 17, 2021

@ewjoachim, I've implemented your requested changes on a branch in my fork. See the PR over in my fork, for just this subset of the changes, here: bskinn#1


Separately, do you have any further thoughts on the web view showing, e.g., pip install foo --pre versus pip install foo==1.2.3b17? While latest would be pretty easy (just redirect to project/{name}), I think that latest-stable and latest-unstable are problematic.

Unless I miss my guess, actually implementing display of pip install foo --pre for latest-unstable on the web view would require addition of new frontend logic ... no idea how desirable that is, for something that's so specific to this new feature.

For latest-stable, I don't know if there's even a pip invocation that matches the behavior.

@bskinn
Copy link
Contributor Author

bskinn commented May 18, 2021

Ok, @ewjoachim, the auxiliary PR is merged. Ready for review starting with ee873dd.

(Not sure what's up with the CI error -- by my read of requirements/main.txt, the google-api-core requirement line looks to be properly formed, with a == version spec.)

@di
Copy link
Member

di commented May 18, 2021

The error is due to pypa/pip#9644

@bskinn bskinn marked this pull request as draft May 18, 2021 17:31
Should be removed before merging pypi#8615
@bskinn
Copy link
Contributor Author

bskinn commented May 18, 2021

Marked back as draft to remind about reverting c6f664a before merge

@di
Copy link
Member

di commented May 18, 2021

@bskinn c6f664a should be unnecessary now that #9543 is merged.

@bskinn bskinn marked this pull request as ready for review May 19, 2021 02:48
@ofek
Copy link
Contributor

ofek commented Aug 18, 2021

Any update on this?

@bskinn
Copy link
Contributor Author

bskinn commented Aug 18, 2021

@ofek Given the extensive discussion ongoing about modernizing/standardizing the JSON API (see, e.g., here), I'm guessing this PR is on hold. (AFAIK the current JSON API is technically in legacy status.)

If the feature set is still desired after all the turnover, I expect I'll need to revise the PR to fit into the new, standardized API, once it's finalized and implemented. (I haven't looked closely at the new PEP draft; it's possible that the functionality of this PR is already part of that plan, but I doubt it since the stated goal was just to PEPify the current API.)

@bskinn
Copy link
Contributor Author

bskinn commented Jun 25, 2022

With the launch of the PEP 691 JSON API, are the semantics of this PR relevant any more? I haven't looked closely at what PEP691 provides in terms of version aliases.

If so, and if the features are still of interest, I'll be happy to reimplement under the PEP691 rubric.

Probably would close this PR and start from scratch.

@bskinn
Copy link
Contributor Author

bskinn commented Jun 25, 2022

<looks closer> Oh. PEP691 deals with a JSON simple API. That's ~unrelated to this, isn't it?

@bskinn bskinn requested a review from a team as a code owner February 22, 2024 19:07
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.

API for fetching latest and stable versions
5 participants