Skip to content
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

Use Connect instead of gRPC #155

Merged
merged 3 commits into from
May 3, 2024
Merged

Use Connect instead of gRPC #155

merged 3 commits into from
May 3, 2024

Conversation

dylan-bourque
Copy link

This PR swaps gRPC for Connect in both the server and CLI implementations. For compatibility with running instances of the Perseus service, we enable the gRPC wire protocol for the client. We have also replaced grpc-gateway with Vanguard (connectrpc.com/vanguard) for JSON/REST support in the server's API.

I have done compatibility testing using an old (gRPC) client hitting a new (Connect) server, a new client hitting an old server, and a new client hitting a new server.

The generated metrics are slightly different due to differences between the gRPC and Connect runtimes, but the things being monitored are the same.

@dylan-bourque dylan-bourque requested review from bndw and alee792 April 25, 2024 16:17
@dylan-bourque dylan-bourque requested review from a team and removed request for a team, bndw and alee792 April 25, 2024 16:33
Makefile Outdated Show resolved Hide resolved
buf.gen.yaml Outdated Show resolved Hide resolved
client_config.go Outdated Show resolved Hide resolved
findpath.go Outdated Show resolved Hide resolved
@dylan-bourque
Copy link
Author

Ran git pull --rebase origin main to sync with unrelated Dependabot updates and force-pushed

Copy link
Contributor

@bndw bndw left a comment

Choose a reason for hiding this comment

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

LGTM. Please update the README section on service architecture to reflect the change from grpc-gateway -> connect.

Copy link

@alee792 alee792 left a comment

Choose a reason for hiding this comment

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

Wrote out a few comments but realized I was more reviewing and asking questions about the connect API and not your actual code lol. Looks great!

Dylan Bourque added 3 commits May 3, 2024 13:03
add code generation using `protoc-gen-connect-go`
replace gRPC server code w/ equivalnt Connect code
replace grpc-gateway w/ Vanguard
replace gRPC client usage in the CLI with Connect client
remove gRPC code generation
updated README.md to reflect the switch to Connect
also updated bufbuild/httplb to v0.3.0
@dylan-bourque dylan-bourque force-pushed the feat/use-connectrpc branch from aac14e1 to c43e27f Compare May 3, 2024 18:04
@dylan-bourque dylan-bourque merged commit 70cf336 into main May 3, 2024
3 checks passed
@dylan-bourque dylan-bourque deleted the feat/use-connectrpc branch May 3, 2024 18:06
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.

4 participants