-
Notifications
You must be signed in to change notification settings - Fork 77
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
Update license headers / Format codes #2780
Conversation
Signed-off-by: Vdaas CI <vald@vdaas.org>
Deploying vald with Cloudflare Pages
|
📝 WalkthroughWalkthroughThe changes involve modifications to the documentation and comments across several proto files in the Changes
Suggested labels
Suggested reviewers
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 (
|
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
🧹 Outside diff range and nitpick comments (4)
apis/proto/v1/vald/upsert.proto (1)
Line range hint
49-54
: Minor formatting inconsistency in error description.The period is missing at the end of the "ALREADY_EXISTS" error description, while other descriptions end with periods.
- | ALREADY_EXISTS | Requested pair of ID and vector is already inserted | Change request payload or nothing to do if update is unnecessary. | + | ALREADY_EXISTS | Requested pair of ID and vector is already inserted. | Change request payload or nothing to do if update is unnecessary. |apis/proto/v1/vald/remove.proto (1)
89-89
: Fix formatting in troubleshooting table.The table formatting is inconsistent, and the period is missing at the end of the sentence.
- // | INTERNAL | Target Vald cluster or network route has some critical error. | Check target Vald cluster first and check network route including ingress as second. + // | INTERNAL | Target Vald cluster or network route has some critical error. | Check target Vald cluster first and check network route including ingress as second. |apis/proto/v1/vald/search.proto (1)
Line range hint
386-386
: Fix comment formatting.Remove the extra forward slash in the comment line.
- // // --- + // ---apis/proto/v1/vald/index.proto (1)
Line range hint
33-61
: Consider standardizing RPC request/response naming in future updates.While the current naming works well, consider adopting more standard gRPC naming conventions in future updates:
- Request types:
{RPC}Request
(e.g.,IndexDetailRequest
)- Response types:
{RPC}Response
(e.g.,IndexDetailResponse
)This is a non-blocking suggestion for future consideration.
🧰 Tools
🪛 buf (1.47.2)
34-36: "payload.v1.Empty" is used as the request or response type for multiple RPCs.
(RPC_REQUEST_RESPONSE_UNIQUE)
34-34: RPC request type "Empty" should be named "IndexInfoRequest" or "IndexIndexInfoRequest".
(RPC_REQUEST_STANDARD_NAME)
34-34: RPC response type "Count" should be named "IndexInfoResponse" or "IndexIndexInfoResponse".
(RPC_RESPONSE_STANDARD_NAME)
40-42: "payload.v1.Empty" is used as the request or response type for multiple RPCs.
(RPC_REQUEST_RESPONSE_UNIQUE)
40-40: RPC request type "Empty" should be named "IndexDetailRequest" or "IndexIndexDetailRequest".
(RPC_REQUEST_STANDARD_NAME)
40-40: RPC response type "Detail" should be named "IndexDetailResponse" or "IndexIndexDetailResponse".
(RPC_RESPONSE_STANDARD_NAME)
46-46: RPC request type "Empty" should be named "IndexStatisticsRequest" or "IndexIndexStatisticsRequest".
(RPC_REQUEST_STANDARD_NAME)
46-46: RPC response type "Statistics" should be named "IndexStatisticsResponse" or "IndexIndexStatisticsResponse".
(RPC_RESPONSE_STANDARD_NAME)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apis/proto/v1/vald/filter.proto
(5 hunks)apis/proto/v1/vald/index.proto
(1 hunks)apis/proto/v1/vald/insert.proto
(4 hunks)apis/proto/v1/vald/object.proto
(3 hunks)apis/proto/v1/vald/remove.proto
(7 hunks)apis/proto/v1/vald/search.proto
(16 hunks)apis/proto/v1/vald/update.proto
(4 hunks)apis/proto/v1/vald/upsert.proto
(4 hunks)
✅ Files skipped from review due to trivial changes (3)
- apis/proto/v1/vald/object.proto
- apis/proto/v1/vald/filter.proto
- apis/proto/v1/vald/update.proto
🧰 Additional context used
🪛 buf (1.47.2)
apis/proto/v1/vald/index.proto
40-42: "payload.v1.Empty" is used as the request or response type for multiple RPCs.
(RPC_REQUEST_RESPONSE_UNIQUE)
40-40: RPC request type "Empty" should be named "IndexDetailRequest" or "IndexIndexDetailRequest".
(RPC_REQUEST_STANDARD_NAME)
40-40: RPC response type "Detail" should be named "IndexDetailResponse" or "IndexIndexDetailResponse".
(RPC_RESPONSE_STANDARD_NAME)
🔇 Additional comments (8)
apis/proto/v1/vald/insert.proto (2)
Line range hint 47-54
: LGTM! Well-structured troubleshooting guidance.
The troubleshooting table provides clear, actionable guidance for common error scenarios.
Line range hint 93-97
: LGTM! Important warning about gRPC message size limitations.
The notice about gRPC message size limitations is a valuable addition for preventing potential runtime issues.
apis/proto/v1/vald/upsert.proto (1)
Line range hint 96-100
: LGTM! Consistent warning about gRPC limitations.
The gRPC message size limitation notice is consistent with other multi-operation endpoints.
apis/proto/v1/vald/remove.proto (2)
Line range hint 65-70
: LGTM! Clear explanation of timestamp-based removal.
The notice provides valuable insight into the AND condition behavior for multiple timestamps.
Line range hint 130-134
: LGTM! Consistent warning about gRPC limitations.
The gRPC message size limitation notice is consistent with other multi-operation endpoints.
apis/proto/v1/vald/search.proto (2)
46-48
: LGTM! Documentation improvements enhance troubleshooting guidance.
The added troubleshooting sections across all RPC methods provide clear, consistent, and helpful guidance for error resolution.
Also applies to: 78-80, 111-113, 139-141, 170-172, 206-208, 237-239, 270-272, 303-305, 331-333, 362-364, 398-400
153-156
: LGTM! Important notice about gRPC message size limitations.
The added notices for multi-request methods appropriately warn users about potential gRPC message size limitations.
Also applies to: 189-192, 345-348, 381-384
apis/proto/v1/vald/index.proto (1)
37-37
: LGTM! Formatting changes improve readability.
The adjustment of blank lines between RPC definitions enhances code consistency.
Also applies to: 43-43
[CHATOPS:HELP] ChatOps commands.
|
Update license headers / Format Go codes and YAML files.
Summary by CodeRabbit
Filter
,Index
,Insert
,Object
,Remove
,Search
,Update
, andUpsert
.