Show a warning message if versions are mismatched in Worker <-> Scheduler communication
#1265
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I hope you to review this. cc: @MarcusSorealheis, @allada, @aaronmondal |
When `Worker` is connecting to the `Scheduler`, both sides should express which version of NativeLink they are using in the metadata and if they are not matched, they should throw a warning. WorkerAPI protobuf is updated to include version information for both `Worker` and `Scheduler` in the initial request and the response to that initial request. `CARGO_PKG_VERSION` environment varaible is used to get the current NativeLink version - which is how `clap` library gets version by itself. Notice that `CARGO_PKG_VERSION` environment variable is only set when the NativeLink is built with `cargo` not `bazel`. Tests are updated to include version information for both `Worker` and `Scheduler`.
aleksdmladenovic
force-pushed
the
handle-version-mismatch
branch
from
c666cd7 to
dd3ae91
Compare
|
FYI, just shortened this commit message to follow the contribution rules. |
allada
reviewed
Aug 16, 2024
There was a problem hiding this comment.
Overall I like this approach, however I'm currently working on a PR to invert the direction of Worker<->Scheduler communication, which will make this proto to be completely be removed.
Reviewable status: 0 of 2 LGTMs obtained, and 0 of 15 files reviewed, and pending CI: windows-2022 / stable
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
When
Workeris connecting to theScheduler, both sides should express which version of NativeLink they are using in the metadata and if they are not matched, they should throw a warning. WorkerAPI protobuf is updated to include version information for bothWorkerandSchedulerin the initial request and the response to that initial request.CARGO_PKG_VERSIONenvironment varaible is used to get the current NativeLink version - which is howclaplibrary gets version by itself. Notice thatCARGO_PKG_VERSIONenvironment variable is only set when the NativeLink is built withcargonotbazel.Tests are updated to include version information for both
WorkerandScheduler.Fixes #1253
Type of change
Please delete options that aren't relevant.
not work as expected)
How Has This Been Tested?
Please also list any relevant details for your test configuration
Checklist
bazel test //...passes locallygit amendsee some docsThis change is