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

fix!: address all gRPC deprecations #22

Merged
merged 1 commit into from
Jul 8, 2024

Conversation

maxbrunet
Copy link
Contributor

@maxbrunet maxbrunet commented Jul 6, 2024

This addresses all deprecation warnings from grpc-go in tests and examples (e.g. use NewClient instead of DialContext, the context from DialContext was only useful if the grpc.WithBlock() option was used, this is also deprecated)

The only exceptions is the change in proxy/codec.go to implement encoding.Codec instead grpc.Codec which is a breaking change.

The clients must now use grpc.NewClient with the grpc.WithDefaultCallOptions(grpc.ForceCodec(proxy.Codec())) option. And servers must now use grpc.NewServer encoding.RegisterCodec(proxy.Codec()) grpc.ForceServerCodec(proxy.Codec()) option.

Happy to look into a solution without breaking change if it is not acceptable :)

Closes #13

@maxbrunet maxbrunet force-pushed the fix/grpc-deprecations branch 2 times, most recently from 5348131 to 763552b Compare July 6, 2024 18:14
Copy link
Member

@DmitriyMV DmitriyMV left a comment

Choose a reason for hiding this comment

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

I think we can remove encoding.RegisterCodec(proxy.Codec()) entirely.

proxy/codec.go Outdated Show resolved Hide resolved
proxy/examples_test.go Outdated Show resolved Hide resolved
proxy/examples_test.go Outdated Show resolved Hide resolved
proxy/examples_test.go Show resolved Hide resolved
proxy/handler_one2many_test.go Show resolved Hide resolved
proxy/handler_one2one_test.go Show resolved Hide resolved
@maxbrunet maxbrunet force-pushed the fix/grpc-deprecations branch 2 times, most recently from 6828407 to 12d908a Compare July 8, 2024 17:54
@maxbrunet maxbrunet requested a review from DmitriyMV July 8, 2024 17:55
This addresses all deprecation warnings from `grpc-go` in tests and examples
(e.g. use `NewClient` instead of `DialContext`, the context from `DialContext`
was only useful if the [`grpc.WithBlock()`] option was used,
this is also deprecated)

The only exceptions is the change in `proxy/codec.go` to implement
`encoding.Codec` instead `grpc.Codec` which is a breaking change.

The clients must now use `grpc.NewClient` with the
`grpc.WithDefaultCallOptions(grpc.ForceCodec(proxy.Codec()))` option.
And servers must now use `grpc.NewServer` `encoding.RegisterCodec(proxy.Codec())`
`grpc.ForceServerCodec(proxy.Codec())` option.

[`grpc.WithBlock()`]: https://pkg.go.dev/google.golang.org/grpc#WithBlock

Signed-off-by: Maxime Brunet <max@brnt.mx>
Signed-off-by: Dmitriy Matrenichev <dmitry.matrenichev@siderolabs.com>
@DmitriyMV
Copy link
Member

Updated the commit and commit/PR message.

@DmitriyMV
Copy link
Member

/m

@talos-bot talos-bot merged commit ec3b59c into siderolabs:main Jul 8, 2024
13 checks passed
@DmitriyMV
Copy link
Member

@maxbrunet congrats this is now merged.

@maxbrunet maxbrunet deleted the fix/grpc-deprecations branch July 9, 2024 16:55
@maxbrunet
Copy link
Contributor Author

Thank you @DmitriyMV! Any chance you could tag a release, please?

@DmitriyMV
Copy link
Member

@maxbrunet https://github.com/siderolabs/grpc-proxy/releases/tag/v0.4.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

StreamDirector definition in README.md is probably outdated
3 participants