Skip to content

feat: add json schema generation for CasConfig via schemars #1640

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Pegasust
Copy link

@Pegasust Pegasust commented Mar 24, 2025

Description

Add JSON schema generation as a runnable binary target

cargo run --bin build-schema --features dev-schema --package nativelink-config

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update: I'm unsure if this is needed, can amend to this PR if needed.

How Has This Been Tested?

I made this change and just run cargo run --bin build-schema --features dev-schema --package nativelink-config, everything builds as expected and a //nativelink_config.schema.json is created.

I eyeballed the generated schema everything looks good enough for me, except for some cases like StoreRef or GrpcEndpoint would benefit from a soft validation regex, and/or some schema documentation bit that it has deserialize_with = "convert_string_with_shellexpand". Nevertheless, I shoulder upon existing docstrings for its detailed documentation and choose to not pursuit further modification to wordings I don't have deep knowledge upon.

Checklist

  • Updated documentation if needed
  • Tests added/amended
  • bazel test //... passes locally: Probably not needed, was not in capacity to set this up locally. I gated my change under crate-local nativelink-config dev-schema to ensure this doesn't break existing workflow. This commit does add a little SBOM burden.
  • PR is contained in a single commit, using git amend see some docs

This change is Reviewable

@CLAassistant
Copy link

CLAassistant commented Mar 24, 2025

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@aaronmondal aaronmondal left a comment

Choose a reason for hiding this comment

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

Nice! Something similar has been in-flight for a while in #1477. That PR contains many changes which are slowly making it into main.

One standalone usecase I could think of regarding schema generation (other than the CRD generation from the above PR) is that it makes it straightforward to identify breaking changes. Could you create a bazel target (so that we can run something like bazel run nativelink-config//:generate-schema instead of the cargo invocation) and a diff_test to check whether the schema has changed? This is similar to what we do in the nativelink-proto directory to detect changes to generated gRPC sources.

Reviewable status: 0 of 2 LGTMs obtained, and 0 of 8 files reviewed

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.

3 participants