-
Notifications
You must be signed in to change notification settings - Fork 694
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
Protovalidate interceptor cleanup, Go version bump #721
Protovalidate interceptor cleanup, Go version bump #721
Conversation
19af624
to
839d345
Compare
Follow up questions:
|
// UnaryServerInterceptor returns a new unary server interceptor that validates incoming messages. | ||
// If the request is invalid, clients may access a structured representation of the validation failure as an error detail. | ||
func UnaryServerInterceptor(validator *protovalidate.Validator, opts ...Option) grpc.UnaryServerInterceptor { | ||
func UnaryServerInterceptor(validator Validator, opts ...Option) grpc.UnaryServerInterceptor { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually a breaking change, and I'm not sure we need to make this change? If you want to propose this, please open an issue. If you remove this change I'm happy to merge the rest of this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed this commit.
No, I'd rather users create their own interceptors for this sort of thing.
For outgoing requests? For responses from servers? Not sure where that would go. |
According to https://github.com/grpc-ecosystem/go-grpc-middleware?tab=readme-ov-file#prerequisites, three latest versions are supported. Go 1.23 is out, so we can bump minimum required version to 1.21 and start using "slices" and "log/slog" from stdlib.
839d345
to
a415b61
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks for your contribution! |
Thank you! |
Changes
Please see individual commits.
Verification
Unit tests. No significant changes.