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

Add new endpoint giving simple rustdoc status for a version #2147

Merged
merged 5 commits into from
Jul 8, 2023

Conversation

Nemo157
Copy link
Member

@Nemo157 Nemo157 commented Jun 9, 2023

As discussed in #2144, this adds a new endpoint for simply requesting whether docs are available for a specific version, without exposing other details about our builds.

I've also fixed the metrics recording /builds.json requests as being "static resource" stopping us from being able to see how many requests we get, that will let us verify whether there's any other clients hitting that endpoint once shields.io/crates.io/lib.rs have migrated over.

@Nemo157 Nemo157 requested a review from syphar June 9, 2023 12:08
@github-actions github-actions bot added the S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed label Jun 9, 2023
@Nemo157 Nemo157 force-pushed the simple-status-api branch 2 times, most recently from 0e9b8c3 to 469d0c9 Compare June 9, 2023 12:15
src/web/status.rs Outdated Show resolved Hide resolved
Copy link
Member

@syphar syphar left a comment

Choose a reason for hiding this comment

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

Thank you for working on this!

@syphar syphar added S-waiting-on-author Status: This PR is incomplete or needs to address review comments and removed S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed labels Jun 16, 2023
@Nemo157 Nemo157 added S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed and removed S-waiting-on-author Status: This PR is incomplete or needs to address review comments labels Jun 30, 2023
Copy link
Member

@syphar syphar left a comment

Choose a reason for hiding this comment

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

Last two comments ;)

src/web/status.rs Outdated Show resolved Hide resolved
src/web/status.rs Show resolved Hide resolved
@syphar
Copy link
Member

syphar commented Jul 3, 2023

@Nemo157 after manually testing, I saw one thing:

we do seem to follow semver logic, so a request to /crate/itertools/0.10.0/status.json, where 0.10.0 doesn't exist, would lead to the endpoint returning the status for 0.10.5 (the latest semver for this).

Seeing the two use-cases I can imagine:

  1. shields.io will request /latest/ (?)
  2. crates.io will request a specific version

Perhaps I'm missing another case?

for the crates.io case I could imagine some edge-cases where the status for a version would be returned that crates.io didn't request. Though this could currently only happen when we're missing a version that is not the latest of that semver bracket.

Would crates.io check the returned version? Or should we perhaps return a redirect similar to the builds.json endpoint? ( crates.io would need to ignore that one too then).

@Nemo157
Copy link
Member Author

Nemo157 commented Jul 3, 2023

Huh, interesting, likely the only reason that crates.io doesn't make that mistake right now is that the redirect is lacking a CORS header. Testing on https://crates.io/crates/stylish-core/0.1.0 (which isn't on docs.rs because of weird reasons around cyclic dependencies, but 0.1.1 is):

Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at https://docs.rs/crate/stylish-core/0.1.0/builds.json. (Reason: CORS header ‘Access-Control-Allow-Origin’ missing). Status code: 302.

I could make it only support exact or latest, and return an error on semver matches still, or when changing crates.io to use this endpoint make it do a request to /={version}/.

@syphar
Copy link
Member

syphar commented Jul 4, 2023

I could make it only support exact or latest, and return an error on semver matches still, or when changing crates.io to use this endpoint make it do a request to /={version}/.

Sleeping over it, expecting the latter sounds like a valid approach.

That means the only remaining question (for me, right now) seems to be:

so we want consistency with the other pages, and the builds.json endpoint? Because in these, we have redirects for semver matches.

@syphar syphar merged commit b9b0fe9 into rust-lang:master Jul 8, 2023
6 checks passed
@github-actions github-actions bot added S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it and removed S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed labels Jul 8, 2023
@Nemo157 Nemo157 deleted the simple-status-api branch July 30, 2023 09:57
@Nemo157
Copy link
Member Author

Nemo157 commented Jul 30, 2023

@kornelski given lib.rs' source has gone private, would you be able to migrate to this new API? https://docs.rs/crate/{crate}/{version}/status.json returning a simple { doc_status: boolean, version: string } response. (The version parameter can be a semver matcher, if you want to do exact versioning like I'm recommending for crates.io you should use ={version}, but since you don't have per-version pages I'm not sure if that'd be the best approach).

@syphar syphar removed the S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it label Jul 31, 2023
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.

None yet

2 participants