Skip to content

valkey-glide - feat: Add Valkey GLIDE storage adapter - #2146

Open
EmilBuszylo wants to merge 1 commit into
jaredwray:mainfrom
EmilBuszylo:feat/valkey-glide-adapter
Open

EmilBuszylo wants to merge 1 commit into
jaredwray:mainfrom
EmilBuszylo:feat/valkey-glide-adapter

Conversation

@EmilBuszylo

Copy link
Copy Markdown

Adds @keyv/valkey-glide so Keyv can use the official Valkey GLIDE Node client (@valkey/valkey-glide). This is the path discussed in #1566: keep @keyv/valkey on iovalkey, and land GLIDE as a separate storage adapter.

Please check if the PR fulfills these requirements

  • Followed the Contributing and Code of Conduct guidelines.
  • Tests for the changes have been added (for bug fixes/features) with 100% code coverage.

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Feature: new official storage adapter @keyv/valkey-glide.

Why

GLIDE is the Valkey client with a Rust core, cluster-aware multi-key commands (MGET, UNLINK, …), and read strategies including AZ affinity (readFrom + clientAz). That is not a drop-in for iovalkey, so a dedicated package matches the maintainer note on #1566.

What

  • New package storage/valkey-glide implementing the v6 KeyvStorageAdapter contract (expires via SET + PXAT).
  • Lazy connect: GlideClient.createClient / GlideClusterClient.createClient are async; the first command or getClient() opens the connection. Existing GLIDE clients can be passed in.
  • Constructor accepts a URI (redis://, rediss://, valkey://, valkeys://), a GLIDE config object (cluster, addresses, readFrom, clientAz, useTLS, …), or an existing client.
  • getMany uses GLIDE mget (cluster-aware). clear() / iterator() use SCAN (cluster ClusterScanCursor when needed).
  • Same namespace / useSets key layout as @keyv/valkey. useSets: true is documented as not cluster-safe.
  • Docs: package README, root / keyv README, website adapter overview.

AZ affinity example:

const store = new KeyvValkeyGlide({
  cluster: true,
  addresses: [{ host: 'clustercfg.example.cache.amazonaws.com', port: 6379 }],
  useTLS: true,
  readFrom: 'AZAffinity',
  clientAz: 'us-east-1a',
});

@socket-security

socket-security Bot commented Sep 10, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Added@​valkey/​valkey-glide@​2.5.29910010098100

View full report

@socket-security

socket-security Bot commented Sep 10, 2026

Copy link
Copy Markdown

Warning

Review the following alerts detected in dependencies.

According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.

Action Severity Alert  (click "▶" to expand/collapse)
Warn High
Obfuscated code: npm @protobufjs/float is 90.0% likely obfuscated

Confidence: 0.90

Location: Package overview

From: pnpm-lock.yamlnpm/@valkey/valkey-glide@2.5.2npm/@protobufjs/float@1.0.2

ℹ Read more on: This package | This alert | What is obfuscated code?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Packages should not obfuscate their code. Consider not using packages with obfuscated code.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/@protobufjs/float@1.0.2. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

View full report

Adds @keyv/valkey-glide so Keyv can use the official GLIDE client, including AZ affinity and cluster-aware multi-key commands (jaredwray#1566).

Co-authored-by: Cursor <cursoragent@cursor.com>
@EmilBuszylo
EmilBuszylo force-pushed the feat/valkey-glide-adapter branch from 7015207 to 505b4eb Compare September 14, 2026 07:11
@codecov

codecov Bot commented Sep 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.97638% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.49%. Comparing base (a3404d3) to head (505b4eb).

Files with missing lines Patch % Lines
storage/valkey-glide/src/index.ts 88.97% 28 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##              main    #2146      +/-   ##
===========================================
- Coverage   100.00%   99.49%   -0.51%     
===========================================
  Files           55       56       +1     
  Lines         5281     5535     +254     
  Branches       857      922      +65     
===========================================
+ Hits          5281     5507     +226     
- Misses           0       28      +28     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jaredwray

Copy link
Copy Markdown
Owner

@EmilBuszylo - thanks for doing the work on this. Here is what we need to do before we merge this

Blocking

  1. Fix setMany and deleteMany to use GLIDE batches. Both are Promise.all over per-key commands (storage/valkey-glide/src/index.ts:192 and :217), which exceeds GLIDE's default in-flight limit of 1000. Use Batch for standalone and ClusterBatch for cluster via client.exec(batch, raiseOnError), one SET with PXAT per entry plus SADD when useSets is on, and map results back to the input order. Same for delete() with useSets (line 206), which currently fires two parallel commands per key.
  2. Fix the clear() pattern. Line 266 uses ${prefix}*, so namespace foo also wipes foobar. Change to ${prefix}:* and add a test with two namespaces that share a prefix.
  3. Make iterator() stream. Lines 273 to 288 collect every key with SCAN and then issue a single MGET for all of them. Yield per SCAN page with one MGET per page, in both the standalone and cluster paths. Optionally do the same paging for clear() so it never issues one giant UNLINK.
  4. Bring coverage to 100% and cover the headline features. Uncovered today: the whole cluster SCAN path (lines 353 and 360 to 373), option pass-through (line 421, so readFrom and clientAz are never exercised), URI credentials and database parsing (lines 448 and 456 to 458), the connect-failure path (lines 313 to 314), useSets with no namespace (lines 323 and 332), namespace with a passed-in client (line 62), and the Buffer branches (lines 480 to 484 and 508). Cluster tests should call clear() and iterator() and should assert the { cluster: true, addresses } constructor path works.
  5. Set both new allowBuilds entries to false in pnpm-workspace.yaml. Neither package needs its scripts and install still passes.
  6. Correct the PR checklist. The 100% coverage box is checked but the suite measures 88% statements and 74% branches.

Requested, non-blocking

  1. Tighten the dependency range from ^2.2.0 to ^2.5.2. Only 2.5.2 was tested, and the batch API is what the fix in item 1 relies on.
  2. README: document GLIDE defaults. Request timeout is 250 ms, connection timeout is 2 s, in-flight limit is 1000. Show how to override them through the pass-through options.
  3. README: state platform support. Linux (glibc and musl) and macOS only, no Windows, native binary, roughly 20 MB installed.
  4. README: reword the useSets caveat. It was copied from @keyv/valkey. This adapter has no MULTI, so useSets works on a cluster (verified). It is non-atomic, which is the accurate warning.
  5. Emit error once on connect failure. createClient emits and rethrows (line 313), then set() catches and emits again, so one failed write produces two events.
  6. Remove the synchronous connect emit in the constructor (line 65). No listener can exist yet, so it never reaches anyone.
  7. Pass Buffers through instead of stringifying. toGlideValue (line 475) converts Uint8Array with toString(), which replaces invalid UTF-8 bytes. GLIDE accepts Buffer directly. Low impact because Keyv serializes first, but the current behavior is silently lossy.
  8. Make hasMany use EXISTS. It currently fetches full values through getMany (line 231) just to test presence. A batch of EXISTS avoids moving large values.
  9. Harden parseConnectionUri. decodeURIComponent on a malformed password throws URIError (line 450), a username with no password yields an empty-string password, and query parameters are silently ignored. Either handle these or document them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants