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

Update cloud docs for RBE and Read Only #1322

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

bclark8923
Copy link
Member

@bclark8923 bclark8923 commented Sep 4, 2024

Description

Please include a summary of the changes and the related issue. Please also
include relevant motivation and context.

Fixes # (issue)

Type of change

Please delete options that aren't relevant.

  • Documentation update

How Has This Been Tested?

Please also list any relevant details for your test configuration

Checklist

  • Updated documentation if needed
  • Tests added/amended
  • bazel test //... passes locally
  • PR is contained in a single commit, using git amend see some docs

This change is Reviewable

@bclark8923 bclark8923 force-pushed the rbe-and-other-docs branch 2 times, most recently from 326cdfa to 2eb55fb Compare September 4, 2024 21:27
Copy link
Member

@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.

Mostly LGTM.

  • Let's split off the workflow disables.
  • Make sure to run vale manually on the changed files since the pre-commit hooks don't correctly report warning-level issues.

Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status: 0 of 1 LGTMs obtained, and all files reviewed, and 8 discussions need to be resolved


.github/workflows/nix.yaml line 10 at r1 (raw file):

    branches: [main]
    paths-ignore:
      - 'docs/**'

While we're at it, let's also disable the following workflows for doc-only changes:

  • main
  • native-bazel
  • native-cargo
  • image
  • sanitizers

I'd probably put all of these disables in a different PR as this is not related to the new RBE documentation.


docs/README.md line 26 at r1 (raw file):

for build scripts.

This project requires `bun` and `deno`. The nix flake ships a compatible version of `bun`.

The flake also ships deno.

Suggestion:

compatible versions.

docs/src/content/docs/nativelink-cloud/api-key.mdx line 66 at r1 (raw file):

  --remote_cache=grpcs://cas-account-id.build-faster.nativelink.net \
  --remote_header=x-nativelink-api-key=${{ secrets.NATIVELINK_COM_API_HEADER || 'HARD_CODED_READ_ONLY_KEY_HERE' }} \
  ${{ github.ref == 'refs/heads/main' && ' ' || '--nogenerate_json_trace_profile --remote_upload_local_results=false' }} \

nit: Is the && ' ' part intentional?


docs/src/content/docs/nativelink-cloud/rbe.mdx line 1 at r1 (raw file):

---

Please line-wrap this file at 80 chars where possible to keep it readable on terminal-based editors.


docs/src/content/docs/nativelink-cloud/rbe.mdx line 10 at r1 (raw file):

:::

This guide shows how to configure remote build execution (RBE) for your [Bazel](https://bazel.build/) projects with the

nit


docs/src/content/docs/nativelink-cloud/rbe.mdx line 14 at r1 (raw file):

### Basic Configuration
To enable RBE all you need to do is add the below flag to your Bazel builds:

nit


docs/src/content/docs/nativelink-cloud/rbe.mdx line 19 at r1 (raw file):

This will run your builds on a Ubuntu 22.04 image without any dependencies installed. For most customers we don't expect this to work out of the box as your project most likely depends on installations like GCC/Java/etc. To remedy that, continue with the instructions below to pass in your own images.

nit

Suggestion:

users

docs/src/content/docs/nativelink-cloud/rbe.mdx line 33 at r1 (raw file):

If your images are in your own private image repository, you can pass your repository credentials to allow us to pull your RBE images.

Here's an example with Amazon ECR (Elastic Container Registry) and the AWS CLI:

nit: consider using starlight's tabs here, similar to this:

@bclark8923
Copy link
Member Author

docs/src/content/docs/nativelink-cloud/api-key.mdx line 66 at r1 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

nit: Is the && ' ' part intentional?

Unfortuately yes, we tried "cleaner" implementations but Github didn't like them

Copy link
Member Author

@bclark8923 bclark8923 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 2 of 6 files reviewed, and pending CI: Docs Deployment / ubuntu-24.04, Local / ubuntu-22.04, pre-commit-checks, and 3 discussions need to be resolved


docs/README.md line 26 at r1 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

The flake also ships deno.

Done.


docs/src/content/docs/nativelink-cloud/rbe.mdx line 1 at r1 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

Please line-wrap this file at 80 chars where possible to keep it readable on terminal-based editors.

Done.


.github/workflows/nix.yaml line 10 at r1 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

While we're at it, let's also disable the following workflows for doc-only changes:

  • main
  • native-bazel
  • native-cargo
  • image
  • sanitizers

I'd probably put all of these disables in a different PR as this is not related to the new RBE documentation.

Done.

Copy link
Member Author

@bclark8923 bclark8923 left a comment

Choose a reason for hiding this comment

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

@aaronmondal Updated and ran Vale, lemme know if all good!

Reviewable status: 0 of 1 LGTMs obtained, and 2 of 6 files reviewed, and pending CI: Docs Deployment / ubuntu-24.04, Local / ubuntu-22.04, pre-commit-checks, and 3 discussions need to be resolved

Copy link
Member

@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.

Reviewed 3 of 4 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: 0 of 1 LGTMs obtained, and all files reviewed, and pending CI: Installation / macos-13

Copy link
Member

@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.

:lgtm:

Reviewable status: 1 of 1 LGTMs obtained, and all files reviewed, and pending CI: Installation / macos-13

@bclark8923 bclark8923 enabled auto-merge (squash) September 5, 2024 18:51
@bclark8923 bclark8923 merged commit 96db0cb into TraceMachina:main Sep 5, 2024
20 checks passed
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.

2 participants