Communicate persistent worker protocol to remote execution service #28489
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
This gives remote execution services the information required to correctly handle remote persistent workers that do not use the default protocol. Closes bazelbuild#28405. RELNOTES: The `requires-worker-protocol` execution requirement is now forwarded to remote execution services as a platform property to support intermixing JSON and Proto remote persistent worker protocols across a build.
armandomontanez
force-pushed
the
communicate-worker-protocol
branch
from
197ab47 to
fdf027b
Compare
github-actions
bot
added
team-Remote-Exec
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly implements the forwarding of the requires-worker-protocol execution requirement to the remote execution service. The changes in RemoteExecutionService are clean, and the refactoring to use a builder for platform properties is a good improvement. The test cases in RemoteExecutionServiceTest have been updated appropriately to cover the new functionality. I have no concerns with this change.
| } | ||
| if (spawn.getExecutionInfo().containsKey(ExecutionRequirements.REQUIRES_WORKER_PROTOCOL)) { | ||
| additionalPropertiesBuilder.put( | ||
| ExecutionRequirements.REQUIRES_WORKER_PROTOCOL, |
There was a problem hiding this comment.
Since the worker protocol is specific to Bazel, we could consider prefixing this with bazel-.
2 participants
This gives remote execution services the information required to correctly handle remote persistent workers that do not use the default protocol.
Closes #28405.
RELNOTES: The
requires-worker-protocolexecution requirement is now forwarded to remote execution services as a platform property to support intermixing JSON and Proto remote persistent worker protocols across a build.