Backport PR #2811 to release/v1.7 for add Search_Config validation #2817
Conversation
* add Search_Config validation * change to error function and generate individual errors * add search configuration
Deploying vald with
|
| Latest commit: |
f1cfea5
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a1cd8950.vald.pages.dev |
| Branch Preview URL: | https://backport-release-v1-7-bugfix-6149.vald.pages.dev |
📝 WalkthroughWalkthroughThis pull request introduces enhanced error handling and configuration management across multiple components. A new error variable Changes
Sequence DiagramsequenceDiagram
participant Client
participant Handler
participant ErrorHandler
Client->>Handler: Search Request
Handler->>Handler: Validate Configuration
alt Invalid Configuration
Handler->>ErrorHandler: Generate ErrInvalidSearchConfig
ErrorHandler-->>Handler: Return Error
Handler-->>Client: Return Error
else Valid Configuration
Handler->>Handler: Process Search
Handler-->>Client: Return Search Results
end
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/tools/cli/loadtest/service/search.go (1)
37-41: Consider documenting the default Search_Config values.While the default values (Num=10, Radius=-1, Epsilon=0.1) seem reasonable, adding comments explaining their significance and why these specific values were chosen would improve maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
internal/errors/lb.go(1 hunks)pkg/gateway/lb/handler/grpc/aggregation.go(1 hunks)pkg/gateway/lb/handler/grpc/handler.go(1 hunks)pkg/tools/cli/loadtest/service/search.go(1 hunks)
🔇 Additional comments (3)
internal/errors/lb.go (1)
28-32: LGTM! Well-structured error handling implementation.The new
ErrInvalidSearchConfigerror function follows the package's error handling pattern and provides clear error messages.pkg/gateway/lb/handler/grpc/aggregation.go (1)
68-71: LGTM! Good placement of validation check.The nil check for bcfg is well-placed at the start of the function for early error detection. The error handling is consistent with the package's patterns.
pkg/gateway/lb/handler/grpc/handler.go (1)
441-461: LGTM! Comprehensive error handling with proper tracing.The validation for nil Search_Config is well-implemented with:
- Detailed error message
- Proper error wrapping with BadRequest details
- Consistent tracing and span handling
|
[CHATOPS:HELP] ChatOps commands.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release/v1.7 #2817 +/- ##
===============================================
Coverage ? 23.91%
===============================================
Files ? 547
Lines ? 54586
Branches ? 0
===============================================
Hits ? 13052
Misses ? 40749
Partials ? 785 ☔ View full report in Codecov by Sentry. |
…nic-caused-with-nil-search-config
vankichi
deleted the
backport/release/v1.7/bugfix/grpc/panic-caused-with-nil-search-config
branch
Description
Search_Config is required field for Search request.
However, it might be forget to set a Search_Config value, unintentionally.
Add Search_Config validation gateway and return error response from it.
Related Issue
Versions
Checklist
Special notes for your reviewer
Summary by CodeRabbit
New Features
ErrInvalidSearchConfigto handle invalid search configuration scenarios.Bug Fixes
Improvements