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

Docs.rs API #809

Closed
wants to merge 1 commit into from
Closed

Docs.rs API #809

wants to merge 1 commit into from

Conversation

Kixiron
Copy link
Member

@Kixiron Kixiron commented Jun 3, 2020

Adds an api for shields.io to query docs.rs with.

Users can call /api/v1/badges?crate=<crate> for the latest version of a crate and /api/v1/badges?crate=<crate>&version=<version> for a specific version.
The api returns the crate name, release version, the url of the crate (e.g. https://docs.rs/crate/rcc/0.0.0) and the crate's build status, which can be one of the following:

  • built fully built and available
  • failed the build failed in some way
  • queued the release is in the build queue
  • yanked the release has been yanked
  • not_library the release isn't a library (Not 100% sure about this one since Document a binary? #238 is a thing)

The api is versioned so that we have future-compatibility. I'm not entirely sure how to handle the old badge system, ideally we'd be able to remove it entirely and forward requests, but that's not possible until shields.io has a working thing, which isn't possible until we have a working thing

Copy link
Member

@pietroalbini pietroalbini left a comment

Choose a reason for hiding this comment

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

I don't have the time to do a full review right now, but other than the commit I left I'm not sure this is the right API design. This seems more like a generic API for docs.rs rather than an API just for badges.

I think we could structure the API this way:

  • /-/api/v1/crates/{name}: return information on all the versions

    {
      "name": "crate-name",
      "versions": [
        {
          "version": "1.0.0",
          "docs-url": "https://...",
          "build-status": "built"
        }
      ]
    }
  • /-/api/v1/crates/{name}/versions/{version}: return information on a single version

    {
      "crate": "crate-name",
      "version": "1.0.0",
      "docs-url": "https://...",
      "build-status": "built"
    }

@@ -93,6 +93,7 @@ pub(super) fn build_routes() -> Routes {
"/crate/:name/:version/target-redirect/*",
super::rustdoc::target_redirect_handler,
);
routes.internal_page("/api/v1/badges", super::api::badge_handler_v1);
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't add more reserved prefixes.

I propose all new top-level routes should be prefixed by -, for example:

/-/api/v1/badges

Copy link
Member Author

Choose a reason for hiding this comment

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

What does the - do, I'm not familiar with it

Copy link
Member

Choose a reason for hiding this comment

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

It's just a name that can't be registered on crates.io.

assert!(resp.status().is_success());
assert_eq!(
resp.json::<BadgeV1>()?,
BadgeV1 {
Copy link
Member

Choose a reason for hiding this comment

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

When doing tests on the API I'd match with serde_json::Value (generating the expected result with the json! macro), so we can guarantee the result will be the same regardless of bugs or changes in the serialization code.

@Kixiron Kixiron changed the title Badge API Docs.rs API Jun 6, 2020
@leos
Copy link

leos commented Jun 6, 2020

It would be useful to be able to get a list of the versions available of the docs of any given crate

In general, the RTD api is probably a good starting point for what would be useful on docs.rs.

@Kixiron
Copy link
Member Author

Kixiron commented Jun 7, 2020

I think I've got a decent api design that should be plenty good for the foreseeable future

  • /-/api/v1/crates/{crate_name}/versions See all released versions of a crate, paginated with the limit and offset parameters
{
    "total": 25, // Total number of releases for this crate
    "next": "https://docs.rs/-/api/v1/crates/{crate_name}/versions/?limit=10&offset=20",     // The next page of releases or `null`
    "previous": "https://docs.rs/-/api/v1/crates/{crate_name}/versions/?limit=10&offset=10", // The previous page of releases or `null`
    "latest": "https://docs.rs/-/api/v1/crates/{crate_name}/versions/{latest_version}",      // The latest release of the crate
    "releases": [
        {
            "version": "{release_version}",
            "url": "https://docs.rs/-/api/v1/crates/{crate_name}/versions/{release_version}",
            "status": "{build_status}", // One of "built", "queued" or "failed"
            "yanked": false
        }
    ]
}
  • /-/api/v1/crates/{crate_name}/versions/{version} Information on one specific version of a crate
{
    "version": "{version}",
    "crate_name": "{crate_name}",
    "project_name": "{project_name}",
    "status": "{build_status}", // One of "built", "queued" or "failed"
    "yanked": false,
    "build_time": "2020-06-06T23:31:13+00:00", // RFC 3339 timestamp or `null` if the crate failed to build
    "urls": {
        "documentation": "https://docs.rs/{project_name}/{version}/{crate_name}", // Will be `null` if the crate failed to build
        "crate": "https://docs.rs/crate/{crate_name}/{version}",
        "builds_html": "https://docs.rs/crate/{crate_name}/{version}/builds",
        "builds": "https://docs.rs/crate/{crate_name}/{version}/builds.json",
        "source": "https://docs.rs/crate/{crate_name}/{version}/source" // Will be `null` if the crate failed to build
    },
    "dependencies": [
        "https://docs.rs/-/api/v1/crates/{dep_name}/versions/{dep_version}"
    ]
}

@jyn514 jyn514 added S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed P-low Low priority issues labels Jun 26, 2020
@jyn514 jyn514 added the A-frontend Area: Web frontend label Jul 3, 2020
use params::{Params, Value};
use serde::{Deserialize, Serialize};

/// The json data of a crate release

Choose a reason for hiding this comment

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

Suggested change
/// The json data of a crate release
/// The JSON data of a crate release

NotLibrary,
}

/// The badge api, expects `?crate=<string>` and an optional `version=<string>`, if the version is not

Choose a reason for hiding this comment

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

Suggested change
/// The badge api, expects `?crate=<string>` and an optional `version=<string>`, if the version is not
/// The badge API expects `?crate=<string>` and an optional `version=<string>` – if the version is not

@Kixiron Kixiron added S-needs-design Status: There's a problem here, but no obvious solution; or the solution raises other questions and removed S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed labels Jul 3, 2020
@jyn514 jyn514 added the S-waiting-on-decision This needs a decision from the team on whether it should be merged or not label Aug 21, 2020
@jyn514
Copy link
Member

jyn514 commented Jan 21, 2021

As per #170 (comment), I would prefer we fixed the latest bug rather than introducing a completely new API.

@jyn514 jyn514 closed this Jan 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-frontend Area: Web frontend P-low Low priority issues S-needs-design Status: There's a problem here, but no obvious solution; or the solution raises other questions S-waiting-on-decision This needs a decision from the team on whether it should be merged or not
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants