-
Notifications
You must be signed in to change notification settings - Fork 51
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
Check for slow store to be noop and conditionally replace with fast #670
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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 2 of 2 files at r1.
Reviewable status: 1 of 1 LGTMs obtained
nativelink-store/tests/fast_slow_store_test.rs
line 387 at r1 (raw file):
#[tokio::test] async fn slow_store_replaced_by_fast_store_when_noop() -> Result<(), Error> {
nit: Please reference the ticket in a comment above the test. (ie: regression test)
e2378f3
to
874eb64
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.
Reviewable status: 1 of 1 LGTMs obtained, and pending CI: Analyze (javascript-typescript), Analyze (python), Bazel Dev / ubuntu-22.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Local / ubuntu-22.04, 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, publish-image, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable, zig-cc ubuntu-20.04, zig-cc ubuntu-22.04
nativelink-store/tests/fast_slow_store_test.rs
line 387 at r1 (raw file):
Previously, allada (Nathan (Blaise) Bruer) wrote…
nit: Please reference the ticket in a comment above the test. (ie: regression test)
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: 1 of 1 LGTMs obtained, and pending CI: docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04)
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: complete! 2 of 1 LGTMs obtained
Description
FastSlow store now checks if the slow store is a no-op and replaces it with the fast store if it is.
Fixes #665
Type of change
How Has This Been Tested?
Checks if the slow store is replaced with the fast store if it is configured as a noop.
Checklist
bazel test //...
passes locallygit amend
see some docsThis change is