fix: valkeycompat NewAdapter sets maxp incorrectly #102
Conversation
Add AdapterOption type and WithNodeScaleoutLimit functional option to allow configuring the maximum parallelism for node scaleout operations in NewAdapter. This addresses issue valkey-io#96 where maxp was being set incorrectly without user control. Fixes valkey-io#96 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: majiayu000 <[email protected]>
| // If not set, defaults to runtime.GOMAXPROCS(0). | ||
| func WithNodeScaleoutLimit(limit int) AdapterOption { | ||
| return func(c *Compat) { | ||
| c.maxp = limit |
There was a problem hiding this comment.
We many need to avoid it from being set to less than 1.
|
Fixed. Values less than 1 will now default to 1. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #102 +/- ##
==========================================
+ Coverage 94.72% 95.22% +0.49%
==========================================
Files 99 99
Lines 46876 44109 -2767
==========================================
- Hits 44404 42002 -2402
+ Misses 2090 1723 -367
- Partials 382 384 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #96 by adding the ability to configure the maximum parallelism (maxp) for node scaleout operations in the valkeycompat adapter through a functional options pattern.
Key Changes:
- Introduces
AdapterOptiontype andWithNodeScaleoutLimitoption function for configuringmaxp - Maintains backward compatibility by using variadic options with sensible defaults
- Adds comprehensive test coverage for default behavior, option application, and edge cases
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
valkeycompat/adapter.go |
Implements functional options pattern for NewAdapter, adds WithNodeScaleoutLimit option with validation to ensure values ≥ 1 |
valkeycompat/adapter_option_test.go |
Adds comprehensive test suite covering default behavior, option setting, multiple options, and edge case handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3 participants
Summary
This PR fixes #96
Changes
valkeycompat/adapter.govalkeycompat/adapter_option_test.go