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

Enhance configuration documentation #970

Merged
merged 1 commit into from
Jun 5, 2024

Conversation

blakehatch
Copy link
Member

@blakehatch blakehatch commented Jun 4, 2024

Preview Deployment: https://nativelink-docs-git-fork-blakehat-76e9db-native-link-web-assets.vercel.app/

Description

Enhances the documentation for configurations.

Fixes #958

Type of change

  • This change requires a documentation update

Checklist

  • Updated documentation if needed
  • PR is contained in a single commit, using git amend see some docs

This change is Reviewable

@CLAassistant
Copy link

CLAassistant commented Jun 4, 2024

CLA assistant check
All committers have signed the CLA.

@MarcusSorealheis
Copy link
Collaborator

I think the best solution is for you to have the same top level page you have and to add another nav link, Configuration Breakdown. What do you think?

Something like:

{
    "label": "Configuration Breakdown",
    "position": 2,
    "link": {
      "type": "generated-index",
      "description": "Explore the configuration JSON object."
    }
  }

Copy link
Collaborator

@MarcusSorealheis MarcusSorealheis left a comment

Choose a reason for hiding this comment

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

This is fine. Let's merge this and iterate.

Copy link
Member

@adam-singer adam-singer left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 7 files at r1, all commit messages.
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Cargo Dev / macos-13, Installation / macos-13, Installation / macos-14, Installation / ubuntu-22.04, macos-13, pre-commit-checks, and 7 discussions need to be resolved


nativelink-docs/docs/configuration/Assembling.mdx line 10 at r1 (raw file):

NativeLink uses a JSON file as the configuration format. This section of the
documentation will breakdown the anatomy of this configuration file with step-by-step instructions on its assembly.

While not directly relevant, might be useful for hinting at different configurations can be used for different types of servers. Nativelink can be bound together in a single configuration, a practical production deployment would contain different configurations for the different types of servers. Maybe thats not in this section of documentation, but somewhere this should be identified and explained on the practical level.


nativelink-docs/docs/configuration/Assembling.mdx line 33 at r1 (raw file):

### Store Name

The value of `stores` includes top-level keys, which are user supplied names stores. The following example, defines the `AC_MAIN_STORE`.

Should we also use some language that hints keys can be referenced in other contexts?


nativelink-docs/docs/configuration/Assembling.mdx line 50 at r1 (raw file):

Once the store has been named and its object exists,
the next key is the type of store. The options are `filesystem`, `memory`, `compression`, `dedup`, `fast_slow`, `verify`, and `experimental_s3_store`.

Is there ability to use direct linking to the definition or reference to what filesystem is in this context instead of highlighting it with "`", like the code docs page or source code (ideally not)?


nativelink-docs/docs/configuration/Assembling.mdx line 79 at r1 (raw file):

        "eviction_policy": {
          // 500mb.
          "max_bytes": 500000000,

Friendly names have landed, we should use those for docs da2c4a7


nativelink-docs/docs/configuration/Assembling.mdx line 98 at r1 (raw file):

### Worker Array

The value of `workers` includes a top level array that embeds the worker metadata. This array always begins with the `local` object, which is the only item permitted at this time.

Can we give a one line statement of what being "local" means even if it sounds trivial ?


nativelink-docs/docs/configuration/Assembling.mdx line 114 at r1 (raw file):

### Local Worker Object Members

The Local object has five components, `worker_api_endpoint`, `cas_fast_slow_store`, `upload_action_results`,`work_directory`, and `platform_properties`.

There is a lot to unpack here, should we expand a little bit on what each of them mean with human explained example?


nativelink-docs/docs/configuration/Reference.mdx line 14 at r1 (raw file):

<Tabs>
    <TabItem value="stores" label="Stores" default>
        | Key                                      | Type          | Description                                                                 | Example                                   |

Possible to add code generated docs or code linked html refs?

@blakehatch
Copy link
Member Author

Reviewed 2 of 7 files at r1, all commit messages.
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Cargo Dev / macos-13, Installation / macos-13, Installation / macos-14, Installation / ubuntu-22.04, macos-13, pre-commit-checks, and 7 discussions need to be resolved

nativelink-docs/docs/configuration/Assembling.mdx line 10 at r1 (raw file):

NativeLink uses a JSON file as the configuration format. This section of the
documentation will breakdown the anatomy of this configuration file with step-by-step instructions on its assembly.

While not directly relevant, might be useful for hinting at different configurations can be used for different types of servers. Nativelink can be bound together in a single configuration, a practical production deployment would contain different configurations for the different types of servers. Maybe thats not in this section of documentation, but somewhere this should be identified and explained on the practical level.

nativelink-docs/docs/configuration/Assembling.mdx line 33 at r1 (raw file):

### Store Name

The value of `stores` includes top-level keys, which are user supplied names stores. The following example, defines the `AC_MAIN_STORE`.

Should we also use some language that hints keys can be referenced in other contexts?

nativelink-docs/docs/configuration/Assembling.mdx line 50 at r1 (raw file):

Once the store has been named and its object exists,
the next key is the type of store. The options are `filesystem`, `memory`, `compression`, `dedup`, `fast_slow`, `verify`, and `experimental_s3_store`.

Is there ability to use direct linking to the definition or reference to what filesystem is in this context instead of highlighting it with "`", like the code docs page or source code (ideally not)?

nativelink-docs/docs/configuration/Assembling.mdx line 79 at r1 (raw file):

        "eviction_policy": {
          // 500mb.
          "max_bytes": 500000000,

Friendly names have landed, we should use those for docs da2c4a7

nativelink-docs/docs/configuration/Assembling.mdx line 98 at r1 (raw file):

### Worker Array

The value of `workers` includes a top level array that embeds the worker metadata. This array always begins with the `local` object, which is the only item permitted at this time.

Can we give a one line statement of what being "local" means even if it sounds trivial ?

nativelink-docs/docs/configuration/Assembling.mdx line 114 at r1 (raw file):

### Local Worker Object Members

The Local object has five components, `worker_api_endpoint`, `cas_fast_slow_store`, `upload_action_results`,`work_directory`, and `platform_properties`.

There is a lot to unpack here, should we expand a little bit on what each of them mean with human explained example?

nativelink-docs/docs/configuration/Reference.mdx line 14 at r1 (raw file):

<Tabs>
    <TabItem value="stores" label="Stores" default>
        | Key                                      | Type          | Description                                                                 | Example                                   |

Possible to add code generated docs or code linked html refs?

Do keep in mine that Assembling is a direct rip from a README in the codebase. I'd rather fix these is a separate PR and sync them instead of making the content diverge here, so we have more ability to automate docs in the future.

@blakehatch
Copy link
Member Author

Reviewed 2 of 7 files at r1, all commit messages.
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Cargo Dev / macos-13, Installation / macos-13, Installation / macos-14, Installation / ubuntu-22.04, macos-13, pre-commit-checks, and 7 discussions need to be resolved

nativelink-docs/docs/configuration/Assembling.mdx line 10 at r1 (raw file):

NativeLink uses a JSON file as the configuration format. This section of the
documentation will breakdown the anatomy of this configuration file with step-by-step instructions on its assembly.

While not directly relevant, might be useful for hinting at different configurations can be used for different types of servers. Nativelink can be bound together in a single configuration, a practical production deployment would contain different configurations for the different types of servers. Maybe thats not in this section of documentation, but somewhere this should be identified and explained on the practical level.
nativelink-docs/docs/configuration/Assembling.mdx line 33 at r1 (raw file):

### Store Name

The value of `stores` includes top-level keys, which are user supplied names stores. The following example, defines the `AC_MAIN_STORE`.

Should we also use some language that hints keys can be referenced in other contexts?
nativelink-docs/docs/configuration/Assembling.mdx line 50 at r1 (raw file):

Once the store has been named and its object exists,
the next key is the type of store. The options are `filesystem`, `memory`, `compression`, `dedup`, `fast_slow`, `verify`, and `experimental_s3_store`.

Is there ability to use direct linking to the definition or reference to what filesystem is in this context instead of highlighting it with "", like the code docs page or source code (ideally not)? _[nativelink-docs/docs/configuration/Assembling.mdx` line 79 at r1](https://reviewable.io/reviews/TraceMachina/nativelink/970#-Nz_YuRcDdddzZBjMNZc:-Nz_YuRcDdddzZBjMNZd:bhgmryr) (raw file):_

        "eviction_policy": {
          // 500mb.
          "max_bytes": 500000000,

Friendly names have landed, we should use those for docs da2c4a7
nativelink-docs/docs/configuration/Assembling.mdx line 98 at r1 (raw file):

### Worker Array

The value of `workers` includes a top level array that embeds the worker metadata. This array always begins with the `local` object, which is the only item permitted at this time.

Can we give a one line statement of what being "local" means even if it sounds trivial ?
nativelink-docs/docs/configuration/Assembling.mdx line 114 at r1 (raw file):

### Local Worker Object Members

The Local object has five components, `worker_api_endpoint`, `cas_fast_slow_store`, `upload_action_results`,`work_directory`, and `platform_properties`.

There is a lot to unpack here, should we expand a little bit on what each of them mean with human explained example?
nativelink-docs/docs/configuration/Reference.mdx line 14 at r1 (raw file):

<Tabs>
    <TabItem value="stores" label="Stores" default>
        | Key                                      | Type          | Description                                                                 | Example                                   |

Possible to add code generated docs or code linked html refs?

Do keep in mine that Assembling is a direct rip from a README in the codebase. I'd rather fix these is a separate PR and sync them instead of making the content diverge here, so we have more ability to automate docs in the future.

Also going to add code generated docs to the links once @aaronmondal lands the rustdoc PR. That will also be separate.

@blakehatch
Copy link
Member Author

This is fine. Let's merge this and iterate.

I'd rather keep them coupled together but either way probably a separate PR.

Copy link
Member Author

@blakehatch blakehatch left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Installation / ubuntu-22.04, and 6 discussions need to be resolved


nativelink-docs/docs/configuration/Assembling.mdx line 33 at r1 (raw file):

Previously, adam-singer (Adam Singer) wrote…

Should we also use some language that hints keys can be referenced in other contexts?

Maybe I'm not forseeing +90% of users configuring that granularly anyways.


nativelink-docs/docs/configuration/Assembling.mdx line 50 at r1 (raw file):

Previously, adam-singer (Adam Singer) wrote…

Is there ability to use direct linking to the definition or reference to what filesystem is in this context instead of highlighting it with "`", like the code docs page or source code (ideally not)?

Also can add in separate PR.


nativelink-docs/docs/configuration/Assembling.mdx line 79 at r1 (raw file):

Previously, adam-singer (Adam Singer) wrote…

Friendly names have landed, we should use those for docs da2c4a7

Will fix in separate config PR.


nativelink-docs/docs/configuration/Assembling.mdx line 98 at r1 (raw file):

Previously, adam-singer (Adam Singer) wrote…

Can we give a one line statement of what being "local" means even if it sounds trivial ?

Will fix in separate config PR.


nativelink-docs/docs/configuration/Assembling.mdx line 114 at r1 (raw file):

Previously, adam-singer (Adam Singer) wrote…

There is a lot to unpack here, should we expand a little bit on what each of them mean with human explained example?

Will fix in separate config PR.

Copy link
Member Author

@blakehatch blakehatch left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: 1 of 1 LGTMs obtained, and pending CI: Installation / ubuntu-22.04

@blakehatch blakehatch merged commit c0c09ed into TraceMachina:main Jun 5, 2024
28 checks passed
Copy link
Member

@allada allada left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 7 files at r1.
Reviewable status: 1 of 1 LGTMs obtained, and 4 discussions need to be resolved


-- commits line 2 at r1:
nit: Please be more descriptive.


nativelink-docs/docs/configuration/Assembling.mdx line 10 at r1 (raw file):

NativeLink uses a JSON file as the configuration format. This section of the
documentation will breakdown the anatomy of this configuration file with step-by-step instructions on its assembly.

nit: anatomy is a needlessly large word.


nativelink-docs/docs/configuration/Assembling.mdx line 50 at r1 (raw file):

Once the store has been named and its object exists,
the next key is the type of store. The options are `filesystem`, `memory`, `compression`, `dedup`, `fast_slow`, `verify`, and `experimental_s3_store`.

Can we not list these? This is going to get out of date quickly.


nativelink-docs/docs/configuration/Examples.mdx line 27 at r1 (raw file):

      borderRadius: '10px 10px 10px 10px'
    }}>
```json5

nit: Did we test that all of these still work? This is dangerous and likely going to stop working with breaking changes. We really need a CI system to run these configs to ensure they continue to work.

blakehatch added a commit to blakehatch/nativelink that referenced this pull request Jun 6, 2024
This pull request was closed.
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.

Configuration Documentation Enhancement
5 participants