fix: correct typo stingSliceResults -> stringSliceResults#346
fix: correct typo stingSliceResults -> stringSliceResults#346yahyasaqban-lab wants to merge 1 commit into
Conversation
Fixed typo in variable name in store_valkey.go (5 occurrences). Fixes volcano-sh#345 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Keywords which can automatically close issues and at(@) or hashtag(#) mentions are not allowed in commit messages. The list of commits with invalid commit messages:
DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
Welcome @yahyasaqban-lab! It looks like this is your first PR to volcano-sh/agentcube 🎉 |
There was a problem hiding this comment.
Pull request overview
Fixes a typo in the Valkey store implementation (pkg/store/store_valkey.go) by renaming stingSliceResults to stringSliceResults, improving code clarity and avoiding confusion when reading Valkey MGET handling.
Changes:
- Renamed the local variable
stingSliceResults→stringSliceResultsand updated all in-scope references inloadSandboxesBySessionIDs.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request corrects a typo in the variable name stingSliceResults to stringSliceResults within the loadSandboxesBySessionIDs function. The reviewer suggested improving the validation logic for the MGET result size by checking for exact equality against the number of requested keys, which ensures the integrity of positional mapping.
| if len(stringSliceResults) > len(sessionIDKeys) { | ||
| return nil, fmt.Errorf("unexpected MGet result size: %d, param size: %d", len(stringSliceResults), len(sessionIDKeys)) | ||
| } |
There was a problem hiding this comment.
The validation logic for the MGET result size should check for exact equality rather than just whether the result is larger than expected. In Redis/Valkey, MGET always returns a result for every key requested, maintaining the same order. If the lengths differ, it indicates a protocol or communication issue that would make the subsequent positional mapping (e.g., using the index i to reference sessionIDs[i] in the loop below) incorrect. Using != provides a more robust check.
| if len(stringSliceResults) > len(sessionIDKeys) { | |
| return nil, fmt.Errorf("unexpected MGet result size: %d, param size: %d", len(stringSliceResults), len(sessionIDKeys)) | |
| } | |
| if len(stringSliceResults) != len(sessionIDKeys) { | |
| return nil, fmt.Errorf("unexpected MGet result size: %d, expected: %d", len(stringSliceResults), len(sessionIDKeys)) | |
| } |
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #346 +/- ##
==========================================
+ Coverage 47.57% 49.12% +1.55%
==========================================
Files 30 30
Lines 2819 2858 +39
==========================================
+ Hits 1341 1404 +63
+ Misses 1338 1301 -37
- Partials 140 153 +13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
FAUST-BENCHOU
left a comment
FAUST-BENCHOU
left a comment
There was a problem hiding this comment.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
we dont accept this kind of DCO
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hzxuzhonghu The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Summary
Fixed typo in variable name
stingSliceResults->stringSliceResultsinpkg/store/store_valkey.go(5 occurrences).Test plan
Fixes #345
🤖 Generated with Claude Code