-
Notifications
You must be signed in to change notification settings - Fork 14
Enhance gRPC client setup with default call options #941
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
Conversation
…age size handling
WalkthroughA single file modification in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
bootstrap/bootstrap.go (1)
517-521: Verify retry interceptor consistency across spork clients.The fix correctly adds
MaxCallRecvMsgSizeto past spork clients, which will resolve the message size errors. However, the current spork client (lines 496-505) includes bothMaxCallRecvMsgSizeand aretryInterceptorfor handlingResourceExhaustederrors, while past spork clients only receive the message size configuration.Consider adding the retry interceptor to past spork clients as well for consistent error handling behavior.
🔎 Apply this diff to add retry interceptor to past spork clients:
grpcClient, err := grpc.NewClient(host, grpc.WithGRPCDialOptions( grpcOpts.WithDefaultCallOptions(grpcOpts.MaxCallRecvMsgSize(DefaultMaxMessageSize)), + grpcOpts.WithUnaryInterceptor(retryInterceptor( + DefaultResourceExhaustedMaxRetryDelay, + DefaultResourceExhaustedRetryDelay, + )), ), )
m-Peter
left a comment
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.
Nice catch 💯
Closes: #940
Description
Enhance the gRPC client setup by introducing default call options to improve message size handling. This change ensures better management of message sizes during client-server communication.
For contributor use:
masterbranchFiles changedin the Github PR explorerSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.