Start automated aggregation and surfacing of internal documentation with Rustdoc #688

blakehatch · 2024-03-02T01:53:20Z

First PR for leveraging rustdoc to surface existing documentation in the code. This reduces the need for redundant documentation on complex and quickly-evolving systems such as configuration files and stores.

This PR focuses on requested improvements and aggregation of information on configuring Nativelink, as well as establishing rustdoc patterns to use on the rest of the monorepo as it is continuously documented.

The change surfaces the below information from the existing nativelink-config documentation for the rust doc:

Example READMEs (For examples from In-flight PR from @MarcusSorealheis & Kubernetes deployment examples)
- In order to thoroughly document the requested cas.json , scheduler.json, cas.yaml, and scheduler.yaml examples they require updating in one place

After running cargo doc in the repository, this page can be reviewed in the browser at:
file:///Path/to/repo/nativelink/target/doc/nativelink_config/index.html

This change also surfaces existing documentation and mildly enhances with examples for:

nativelink-config/stores.rs -> enum StoreConfig

After running cargo doc in the repository, this page can be reviewed in the browser at:
file:///Users/blakehatch/Projects/nativelink/target/doc/nativelink_config/stores/enum.StoreConfig.html

Upcoming PRs will focus on improving the existing now surfaced and aggregated documentation as well as the formatting of the generated Markdown in nativelink-config/src/lib.rs

#681

Type of change

New feature (non-breaking change which adds functionality)
This change requires a documentation update

This change is

vercel · 2024-03-02T01:53:25Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
nativelink-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 2, 2024 1:53am

aaronmondal

I like that we're starting to move towards more generated documentation and the added config examples are nice.

The way this is currently set up will break our bazel tests though. Note that we doc targets for the individual crates with bazel build nativelink-config:docs and so on. There are also generated doc tests that run as part of the testsuite, e.g. bazel test nativelink-config:doc_test.

I think it would be good to have the option to generate the docs with either Bazel or Cargo, but if we have to choose one we'll have to use Bazel since we don't have the same reliability for clippy's doc checking in our Cargo build.

Reviewed 3 of 4 files at r1, all commit messages.
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Bazel Dev / ubuntu-22.04, Cargo Dev / ubuntu-22.04, Remote / large-ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), macos-13, pre-commit-checks, publish-image, ubuntu-22.04, zig-cc ubuntu-20.04, zig-cc ubuntu-22.04 (waiting on @blakehatch)

nativelink-config/Cargo.toml line 11 at r1 (raw file):

[package.metadata.docs.rs]
readme = "./README.md"

nit: newline

nativelink-config/examples/README.md line 0 at r1 (raw file):
nit: Intentionally empty?

nativelink-config/src/lib.rs line 15 at r1 (raw file):

// limitations under the License.

#![doc = include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/README.md"))]

Viewing this in the docpage seems a bit overwhelming. Could we move the specific config examples to the respective stores?

nativelink-config/src/stores.rs line 42 at r1 (raw file):

    /// Memory store will store all data in a hashmap in memory.
    ///
    /// **Example JSON Config:**

I believe there needs to be an empty line around the code snippet to make sure that it's formatted correctly.

nativelink-config/src/stores.rs line 63 at r1 (raw file):

    ///
    /// **Example JSON Config:**
    /// ```json

nit: ditto

nativelink-config/src/stores.rs line 89 at r1 (raw file):

    ///
    /// **Example JSON Config:**
    /// ```json

nit: ditto

nativelink-config/src/stores.rs line 169 at r1 (raw file):

    ///
    /// **Example JSON Config:**
    /// ```json

nit: ditto

nativelink-config/src/stores.rs line 198 at r1 (raw file):

    /// store before those stores.
    ///
    /// **Example JSON Config:**

nit:ditto

nativelink-config/src/stores.rs line 245 at r1 (raw file):

    ///
    /// **Example JSON Config:**
    /// ```json

nit: ditto

nativelink-config/src/stores.rs line 320 at r1 (raw file):

    ///
    /// **Example JSON Config:**
    /// ```json

nit: ditto

nativelink-config/src/stores.rs line 358 at r1 (raw file):

    ///
    /// ***Example JSON Config:***
    /// ```json

nit: ditto

nativelink-config/src/stores.rs line 393 at r1 (raw file):

    ///
    /// **Example JSON Config:**
    /// ```json

nit: ditto

nativelink-config/src/stores.rs line 401 at r1 (raw file):

    ///               // 10mb.
    ///               "max_bytes": 10000000,
    ///

nit: intentional dots?

nativelink-config/src/stores.rs line 425 at r1 (raw file):

    ///
    /// **Example JSON Config:**
    // ```json

nit: ditto

nativelink-config/src/stores.rs line 444 at r1 (raw file):

    ///
    /// **Example JSON Config:**
    /// ```json

nit: ditto

nativelink-config/src/stores.rs line 461 at r1 (raw file):

    ///
    /// **Example JSON Config:**
    /// ```json

nit: ditto

nativelink-config/src/stores.rs line 490 at r1 (raw file):

    ///
    /// **Example JSON Config:**
    /// ```json

nit: ditto

nativelink-config/src/stores.rs line 508 at r1 (raw file):

    ///
    /// **Example JSON Config:**
    /// ```json

nit: ditto

aaronmondal

Reviewed 1 of 4 files at r1.
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Bazel Dev / ubuntu-22.04, Cargo Dev / ubuntu-22.04, Remote / large-ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), macos-13, pre-commit-checks, publish-image, ubuntu-22.04, zig-cc ubuntu-20.04, zig-cc ubuntu-22.04 (waiting on @blakehatch)

blakehatch · 2024-03-02T17:56:11Z

Yup there goes CI blowing up lol, let me fix I thought rust format would save me 😄

blakehatch

Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Remote / large-ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), pre-commit-checks, publish-image (waiting on @blakehatch)

nativelink-config/examples/README.md line at r1 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

nit: Intentionally empty?

Yes this is the file Marcus is working on for #680

I'm gonna rebase and land this after that's in, this is just there in the interim so there's something to point to.

nativelink-config/src/lib.rs line 15 at r1 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

Viewing this in the docpage seems a bit overwhelming. Could we move the specific config examples to the respective stores?

Yeah I want to restructure this page, just wanted to get everything in one place to start. Will try to find a good place to put everything and maybe take chunks of each README for the docs.

nativelink-config/src/stores.rs line 42 at r1 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

I believe there needs to be an empty line around the code snippet to make sure that it's formatted correctly.

There doesn't as we saw yesterday, rustdoc just only supports rust and text code snippets lol.

nativelink-config/src/stores.rs line 401 at r1 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

nit: intentional dots?

Fixed, was whitespace. Not sure why it's shown like that by reviewable.

MarcusSorealheis

I don't think you should change the config example just yet. I think the more immediate need is for a sections in the docs that enumerates the config options and what all the values mean.

allada

Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Remote / large-ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), pre-commit-checks, publish-image (waiting on @blakehatch)

nativelink-config/src/stores.rs line 65 at r2 (raw file):

    /// ```json
    /// "experimental_s3_store": {
    ///      "region": "eu-north-1",

nit: spacing here vs other examples are inconsistent.

nativelink-config/src/stores.rs line 73 at r2 (raw file):

    ///          "jitter": 0.5,
    ///      },
    ///  "additional_max_concurrent_requests": 10

nit: I think we got rid of this field.

nativelink-config/src/stores.rs line 90 at r2 (raw file):

    /// **Example JSON Config:**
    /// ```json
    /// "verify": {

This example is far to complex, can we do this with just a memory or filesystem store?

nativelink-config/src/stores.rs line 260 at r2 (raw file):

    ///         },
    ///         "slow": {
    ///           "experimental_s3_store": {

nit: Lets not use s3, just stick to memory and/or filesystem in examples.

nativelink-config/src/stores.rs line 371 at r2 (raw file):

    ///     },
    ///     "slow": {
    ///       "experimental_s3_store": {

ditto.

nativelink-config/src/stores.rs line 397 at r2 (raw file):

    /// "shard": {
    ///     "stores": [
    ///         "store": {

This is not right, it's an array with a key-value.

nativelink-config/src/stores.rs line 406 at r2 (raw file):

    ///         },
    ///        },
    ///        "store": {

duplicate.

nativelink-config/src/stores.rs line 473 at r2 (raw file):

    ///     },
    ///     "upper_store": {
    ///       "noop": {}

nit: Add comment here saying this store discards data larger than 128mib.

MarcusSorealheis · 2024-03-05T19:15:49Z

Nit: ```json5

blakehatch · 2024-03-08T00:41:44Z

It seems like the lines in lib.rs pulling in the READMEs with relative paths
#![doc = include_str!("../examples/README.md")]
are causing issues with CI and creating the OCI image.

Any idea on a workaround @aaronmondal?

blakehatch

Yeah putting up a PR for that rn, hopefully they can land relatively close to each other they supplement each other nicely.

Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Remote / large-ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), publish-image, and 9 discussions need to be resolved

nativelink-config/src/stores.rs line 65 at r2 (raw file):