-
Notifications
You must be signed in to change notification settings - Fork 975
fix: reduce config warnings #3018
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: reduce config warnings #3018
Conversation
…ings - Add error return type to HealthTripleServer methods (Resume, SetServingStatus, Shutdown) - Fix RPC method signature validation warnings - Only warn about missing service config when services are actually configured - Improve compatibility with new server API without config files
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3018 +/- ##
===========================================
- Coverage 46.76% 39.63% -7.14%
===========================================
Files 295 457 +162
Lines 17172 38884 +21712
===========================================
+ Hits 8031 15410 +7379
- Misses 8287 22216 +13929
- Partials 854 1258 +404 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Pull Request Overview
This PR fixes RPC method signature validation warnings for health server methods and reduces unnecessary configuration warnings when using the new server API without config files.
- Adds error return values to health server methods (Resume, SetServingStatus, Shutdown) to comply with RPC validation requirements
- Improves configuration handling to avoid warning about missing service configs when no services are configured
- Updates RPC service validation to exclude health helper methods from signature checks
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| protocol/triple/health/healthServer.go | Adds error return values to Resume, SetServingStatus, and Shutdown methods |
| server/server.go | Conditionally shows service config warnings only when services are actually configured |
| common/rpc_service.go | Excludes health helper methods from RPC validation checks |
Comments suppressed due to low confidence (2)
protocol/triple/health/healthServer.go:1
- The SetServingStatus method now returns an error, but this call ignores the return value. The error should be handled or returned to the caller.
/*
protocol/triple/health/healthServer.go:1
- The SetServingStatus method now returns an error, but this call ignores the return value. The error should be handled or returned to the caller.
/*
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
00b0e91 to
7821627
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
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



Description
This PR fixes RPC method signature validation warnings for health server methods and improves server configuration handling.
Problem
The following warnings were being generated due to incorrect RPC method signatures:
These warnings occur because the RPC service validation in
common/rpc_service.gorequires methods to have 1 or 2 return values (with the last one beingerror), but the health server methods had no return values.Additionally, unnecessary warnings about missing service configurations were being shown when using the new server API without config files.
Related to apache/dubbo-go-samples#910