chore: simplify, modernize (Go 1.26), update deps#213
Conversation
Bugs fixed: - kv/driver.go: MinRetryBackoff was set to MaxRetryBackoff (copy-paste) — min always equalled max - kv/tlsconfig.go: AppendCertsFromPEM failure returned nil,nil (silent empty pool) — now returns a real error - kv/tlsconfig.go: os.Stat(conf.RootCa) ran unconditionally outside the RootCa block — cert/key-only TLS always failed; moved inside the guard - kv/driver.go: Expire result discarded in MExpire — errors silently lost - kv/driver.go: len(keys)==0 guards replacing keys==nil (Has, MGet, Set, Delete, TTL) - kv/driver.go: Clear checked FlushDB().Err() twice — simplified to single call Simplify / modernise: - kv/tlsconfig.go: removed dead TLS 1.3 CipherSuites block (Go ignores CipherSuites for 1.3); dropped golang.org/x/sys/cpu import - config.go: removed dead root-package Config/InitDefaults (actual config lives in kv.Config) Deps: go mod tidy; golang.org/x/sys demoted to indirect
|
Warning Review limit reached
More reviews will be available in 54 minutes and 30 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR modernizes the Redis KV plugin by fixing several Redis driver/TLS bugs, simplifying dead TLS cipher-suite logic, and tidying module checksums/dependency classification.
Changes:
- Fixes Redis driver behavior for retry backoff, empty variadic inputs,
MExpireerror propagation, andClearerror handling. - Simplifies TLS configuration and returns a real error when CA PEM parsing fails.
- Removes root-package config code and updates Go module sums/dependency metadata.
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
kv/driver.go |
Fixes Redis option wiring and error handling in KV operations. |
kv/tlsconfig.go |
Simplifies TLS defaults and improves root CA parse error handling. |
config.go |
Deletes the root-package exported config type/defaults. |
go.mod |
Moves golang.org/x/sys to indirect dependencies. |
go.sum |
Adds tidy-generated checksum entry. |
tests/go.sum |
Adds tidy-generated checksum entry for tests module. |
go.work.sum |
Updates workspace checksum metadata. |
Comments suppressed due to low confidence (1)
config.go:1
- Removing the exported root-package
Configtype is a breaking API change for consumers that importgithub.com/roadrunner-server/redis/v6and referenceredis.Config, even if this package no longer uses it internally. Keep the compatibility type (or defer removal to a major-version change) so external users do not get compile-time failures from this cleanup.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #213 +/- ##
==========================================
+ Coverage 73.33% 81.48% +8.14%
==========================================
Files 2 1 -1
Lines 30 27 -3
==========================================
Hits 22 22
+ Misses 6 3 -3
Partials 2 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>
Bug fixes
kv/driver.go:74—MinRetryBackoffwas set toMaxRetryBackoff(copy-paste); min always equalled maxkv/tlsconfig.go:41—AppendCertsFromPEMfailure returnednil, nil(silent empty pool); now returns a real errorkv/tlsconfig.go:48—os.Stat(conf.RootCa)ran unconditionally outside theRootCa != ""guard; cert/key-only TLS always failed; moved inside the blockkv/driver.go:286—Expireresult discarded inMExpire; errors silently lostkv/driver.go—keys == nilguards replaced withlen(keys) == 0(Has,MGet,Set,Delete,TTL)kv/driver.go:334—ClearcalledFlushDB().Err()twice; simplified to a single callSimplify / modernise
kv/tlsconfig.go— removed dead TLS 1.3CipherSuitesblock (Go ignoresCipherSuitesfor 1.3 since Go 1.17); droppedgolang.org/x/sys/cpudirect dependencyconfig.go— removed dead root-packageConfig/InitDefaults(actual config lives inkv.Configand is never read from the root package)Deps
go mod tidy;golang.org/x/sysdemoted to indirect (no longer directly imported)