Skip to content

Conversation

@f0rmiga
Copy link
Contributor

@f0rmiga f0rmiga commented Sep 18, 2021

Description

Adds a local gRPC service for the Build Event Protocol events. It listens on a random free port provided by the OS on localhost, which gets passed to the bazel build command.

Test plan

Check the unit tests with subscribers and assert the test logic is correct.

@f0rmiga f0rmiga mentioned this pull request Sep 18, 2021
f0rmiga referenced this pull request Sep 18, 2021
otherwise it runs the prettier check on commit messages and finds no files
Copy link
Member

@alexeagle alexeagle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great


// Setup sets up the gRPC server.
func (bb *besBackend) Setup(opts ...grpc.ServerOption) error {
lis, err := bb.netListen("tcp", "127.0.0.1:0")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does the :0 mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In UNIX, it means the port will be determined by the OS. It's a way of getting a random available port. In CLIs like this, we don't know the environment it will be executed, therefore, we don't want to pick a static port.

Copy link
Contributor Author

@f0rmiga f0rmiga Sep 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, there's a latency in releasing the resources used by the previous run. Asking for a port from the OS makes it much faster between the CLI executions.

@f0rmiga f0rmiga merged commit 1d3b5bc into main Sep 24, 2021
@f0rmiga f0rmiga deleted the bep branch September 24, 2021 17:02
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.

3 participants