-
Notifications
You must be signed in to change notification settings - Fork 102
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
Added README to docker-compose deployment-example #427
Added README to docker-compose deployment-example #427
Conversation
There was a problem hiding this 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 files reviewed, 1 unresolved discussion (waiting on @blakehatch and @MarcusSorealheis)
deployment-examples/docker-compose/README.md
line 110 at r1 (raw file):
## Useful tips The Docker Compose setup can be easily modified for testing and development. For example, you can change the RUST_LOG environment variable to control the logging level of the services.
nit: Missed a new line here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM content-wise 👍
If you're using the nix flake+direnv it should automatically remove bad newlines and trailing whitespace for you on commit.
You can also set markdownlint.enable = true;
in the tools/pre-commit-hooks.nix
file. When you then enter the flake it'll set up markdownlint as pre-commit hook so that you get all the nit
s i posted on commit check.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 19 unresolved discussions (waiting on @blakehatch and @MarcusSorealheis)
deployment-examples/docker-compose/README.md
line 1 at r1 (raw file):
# Native Link's Docker Compose Deployment
nit: There should be an empty line between headers and content.
deployment-examples/docker-compose/README.md
line 4 at r1 (raw file):
This directory contains a reference/starting point on creating a full Docker Compose deployment of Native Link's cache and remote execution system. ## Docker Setup
nit: Missing empty line.
deployment-examples/docker-compose/README.md
line 6 at r1 (raw file):
## Docker Setup 1. Ensure [Docker](https://docs.docker.com/engine/install/) and [Docker Compose](https://docs.docker.com/compose/install/) are installed on your system. 2. Open terminal and navigate to this directory.
a terminal
deployment-examples/docker-compose/README.md
line 9 at r1 (raw file):
3. Run `docker-compose up -d` to start the services. It will take some time to apply, when it is finished everything should be running. The endpoints are:
nit: Code blocks should be surrounded by empty lines.
deployment-examples/docker-compose/README.md
line 10 at r1 (raw file):
It will take some time to apply, when it is finished everything should be running. The endpoints are:
"Language anonymous" code blocks should still specify txt
or some other language that makes things look pretty.
deployment-examples/docker-compose/README.md
line 16 at r1 (raw file):
As a reference you should be able to compile this project using bazel with something like: ```sh
nit: Missing empty line
deployment-examples/docker-compose/README.md
line 24 at r1 (raw file):
Instances
nit: Missing empty line.
deployment-examples/docker-compose/README.md
line 25 at r1 (raw file):
## Instances All instances use the same Docker image, trace_machina/native-link:latest, built from the Dockerfile located at ./deployment-examples/docker-compose/Dockerfile.
Rewiewable is displaying this and some the next section in cursive. Maybe there are some backticks missing somewhere?
deployment-examples/docker-compose/README.md
line 27 at r1 (raw file):
All instances use the same Docker image, trace_machina/native-link:latest, built from the Dockerfile located at ./deployment-examples/docker-compose/Dockerfile. ### CAS
nit: Missing empty line.
deployment-examples/docker-compose/README.md
line 28 at r1 (raw file):
### CAS The CAS (Content Addressable Storage) service is used as a local cache for the Native Link system. It is configured in the docker-compose.yml file under the native_link_local_cas service.
Cursive here as well. Might be a bug in reviewable or bad backticks somewhere.
deployment-examples/docker-compose/README.md
line 45 at r1 (raw file):
environment: RUST_LOG: ${RUST_LOG:-warn} ports: [ "50051:50051/tcp", "127.0.0.1:50061:50061", "50071:50071/tcp", ]
nit: Let's put these in separate lines for readability:
[
"50051:50051/tcp",
"127.0.0.1:50061:50061",
"50071":50071/tcp",
]
deployment-examples/docker-compose/README.md
line 50 at r1 (raw file):
Scheduler
nit: Missing empty line.
deployment-examples/docker-compose/README.md
line 52 at r1 (raw file):
### Scheduler The scheduler is currently the only single point of failure in the system. We currently only support one scheduler at a time. The scheduler service is responsible for scheduling tasks in the Native Link system. It is configured in the docker-compose.yml file under the native_link_scheduler service.
nit: Missing empty line.
deployment-examples/docker-compose/README.md
line 74 at r1 (raw file):
Workers
nit: Missing empty line.
deployment-examples/docker-compose/README.md
line 76 at r1 (raw file):
### Workers Worker instances are responsible for executing tasks. They are configured in the docker-compose.yml file under the native_link_executor service.
nit: Missing empty line.
deployment-examples/docker-compose/README.md
line 98 at r1 (raw file):
Security
nit: Missing empty line.
deployment-examples/docker-compose/README.md
line 105 at r1 (raw file):
If you decide to stop using this setup, you can use `docker-compose down` to stop and remove all the containers. However, in a non-local setup, additional steps may be required to ensure that all data is securely deleted. ## Future work / TODOs
nit: Missing empty line.
deployment-examples/docker-compose/README.md
line 109 at r1 (raw file):
The Docker Compose setup does not automatically delete old data. This could be implemented with a cleanup service or script. ## Useful tips
nit: Missing empty line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 21 unresolved discussions (waiting on @blakehatch and @MarcusSorealheis)
deployment-examples/docker-compose/README.md
line 6 at r1 (raw file):
## Docker Setup 1. Ensure [Docker](https://docs.docker.com/engine/install/) and [Docker Compose](https://docs.docker.com/compose/install/) are installed on your system. 2. Open terminal and navigate to this directory.
nit: Combine 2 and 3. This line is obvious and directly relates to what happens in 3
deployment-examples/docker-compose/README.md
line 11 at r1 (raw file):
It will take some time to apply, when it is finished everything should be running. The endpoints are:CAS/AC: 0.0.0.0:50051/50071
nit: Separate these two and explain the different ports.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 22 unresolved discussions (waiting on @aaronmondal and @blakehatch)
-- commits
line 2 at r1:
Nit: The commit is living on so I prefer Adds
to beginning a commit message with a past participle.
deployment-examples/docker-compose/README.md
line 45 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
nit: Let's put these in separate lines for readability:
[ "50051:50051/tcp", "127.0.0.1:50061:50061", "50071":50071/tcp", ]
Agreed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 22 unresolved discussions (waiting on @aaronmondal and @blakehatch)
539cfcd
to
af29df1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the input y'all! Will set up nix flake+direnv tomorrow.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @aaronmondal)
Previously, MarcusSorealheis (Marcus Eagan) wrote…
Nit: The commit is living on so I prefer
Adds
to beginning a commit message with a past participle.
Done.
deployment-examples/docker-compose/README.md
line 1 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
nit: There should be an empty line between headers and content.
Done.
deployment-examples/docker-compose/README.md
line 4 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
nit: Missing empty line.
Done.
deployment-examples/docker-compose/README.md
line 6 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
a terminal
Done.
deployment-examples/docker-compose/README.md
line 6 at r1 (raw file):
Previously, allada (Nathan (Blaise) Bruer) wrote…
nit: Combine 2 and 3. This line is obvious and directly relates to what happens in 3
Done.
deployment-examples/docker-compose/README.md
line 9 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
nit: Code blocks should be surrounded by empty lines.
Done.
deployment-examples/docker-compose/README.md
line 10 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
"Language anonymous" code blocks should still specify
txt
or some other language that makes things look pretty.
Done.
deployment-examples/docker-compose/README.md
line 11 at r1 (raw file):
Previously, allada (Nathan (Blaise) Bruer) wrote…
nit: Separate these two and explain the different ports.
Done.
deployment-examples/docker-compose/README.md
line 16 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
nit: Missing empty line
Done.
deployment-examples/docker-compose/README.md
line 24 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
nit: Missing empty line.
Done.
deployment-examples/docker-compose/README.md
line 25 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Rewiewable is displaying this and some the next section in cursive. Maybe there are some backticks missing somewhere?
I tried it in a few markdown viewers and nothing was in cursive so probably just a reviewable bug.
deployment-examples/docker-compose/README.md
line 27 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
nit: Missing empty line.
Done.
deployment-examples/docker-compose/README.md
line 28 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Cursive here as well. Might be a bug in reviewable or bad backticks somewhere.
Ditto above.
deployment-examples/docker-compose/README.md
line 45 at r1 (raw file):
Previously, MarcusSorealheis (Marcus Eagan) wrote…
Agreed
Done.
deployment-examples/docker-compose/README.md
line 50 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
nit: Missing empty line.
Done.
deployment-examples/docker-compose/README.md
line 52 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
nit: Missing empty line.
Done.
deployment-examples/docker-compose/README.md
line 74 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
nit: Missing empty line.
Done.
deployment-examples/docker-compose/README.md
line 76 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
nit: Missing empty line.
Done.
deployment-examples/docker-compose/README.md
line 98 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
nit: Missing empty line.
Done.
deployment-examples/docker-compose/README.md
line 105 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
nit: Missing empty line.
Done.
deployment-examples/docker-compose/README.md
line 109 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
nit: Missing empty line.
Done.
deployment-examples/docker-compose/README.md
line 110 at r1 (raw file):
Previously, steed924 (Steed) wrote…
nit: Missed a new line here.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @blakehatch)
-- commits
line 2 at r2:
Nit: Add
, imperative tone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 1 unresolved discussion
Previously, blakehatch (Blake Hatch) wrote…
Done.
Note that Git's recommendation is present tense, imperative mood: https://git.kernel.org/pub/scm/git/git.git/tree/Documentation/SubmittingPatches?h=v2.36.1#n181
Apart from being a somewhat official recommendation it's also usually the shortest form (Add
vs AddS
vs AddED
). We don't have this documented though, so we can't really expect/enforce this at the moment. In any case we should set a standard for this.
Description
Added README for Docker-Compose deployment example covering setup, configuration, notes & tips.
Fixes #425
Type of change
-[x] Documentation Update
Checklist
This change is