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

Add JSON config examples to store.rs #967

Merged
merged 1 commit into from
Jun 5, 2024

Conversation

blakehatch
Copy link
Contributor

@blakehatch blakehatch commented Jun 4, 2024

Description

Please include a summary of the changes and the related issue. Please also
include relevant motivation and context
#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.

Copy link
Contributor

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

:lgtm:

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: 1 of 1 LGTMs obtained, and pending CI: Bazel Dev / ubuntu-22.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Installation / macos-13, Installation / macos-14, Installation / ubuntu-22.04, Publish image, Publish nativelink-worker-lre-cc, 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, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable

@blakehatch blakehatch merged commit da9399b 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.

:lgtm:

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: 2 of 1 LGTMs obtained, and 2 discussions need to be resolved


nativelink-config/examples/redis.json line 14 at r2 (raw file):

                    "redis://127.0.0.1:6383/",   // Master node 3
                    "redis://127.0.0.1:6384/"   // Master node 3
                    //"redis://172.18.0.2:6379/"  // Master node 3

nit: Remove unused lines.


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

    ///     "backend": {
    ///       "filesystem": {
    ///         "content_path": "~/.cache/nativelink/content_path-ac",

nit: use /tmp/

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.

4 participants