adds Valkey Storage Implementation#5603
Closed
MatthiasHowellYopp wants to merge 1 commit intocrewAIInc:mainfrom
Closed
adds Valkey Storage Implementation#5603MatthiasHowellYopp wants to merge 1 commit intocrewAIInc:mainfrom
MatthiasHowellYopp wants to merge 1 commit intocrewAIInc:mainfrom
Conversation
4fbf22f to
b8c5d27
Compare
5b1f60b to
9b12c62
Compare
9b12c62 to
646c828
Compare
bbcfde3 to
7bf8324
Compare
Contributor
Author
|
@greysonlalonde Hoping someone can review this, thanks. |
2e55991 to
c282ab2
Compare
Jonathan-Improving
suggested changes
May 1, 2026
Jonathan-Improving
left a comment
There was a problem hiding this comment.
Consolidated PR Review — Valkey Storage Implementation
Reviewers: GLIDE API validation, Python code quality / DRY, Security (OWASP methodology)
19 files, +9276 / -904 lines, ~5300 lines of tests
See inline comments for specific findings at the relevant lines.
- 🔴 5 must-fix: FT.SEARCH query injection via
=>token, wrong import paths (ImportError), key construction injection, TLS cert params are silent no-ops, credentials set post-construction - 🟡 7 suggestions: Use
ft.list()notft.info()for index check, N+1 query patterns, duplicated client init, duplicated category parsing (4×), duplicated bytes decoding (5×), dead_retry_operation, pagination fetches all records first - 🟢 7 low items: Positional GLIDE API args, scan cursor type, f-strings in logging, deprecated
datetime.utcnow(), TOCTOU in index creation, no batching for bulk saves,_run_asyncreturnsAny
✅ Security Assessment
- 1 HIGH: FT.SEARCH
=>injection via LLM-inferred scope/category values - 2 MEDIUM: Key construction injection, TLS cert silent no-ops
✅ GLIDE API
- Correct
ft.create(client, ...)/ft.search(client, ...)module-level pattern ✅ - Correct KNN query syntax with filter expressions ✅
- Correct binary embedding handling with numpy float32 ✅
- Import paths need fixing (internal
glide.async_commandsvs stableglide_shared) ❌
✅ What's Done Well
- Comprehensive 5300-line test suite covering CRUD, search, scopes, errors, cache
- Correct cosine distance→similarity conversion
- Lazy client init, connection timeouts, retry logic design
- Thread-safe sync/async bridge with persistent background event loop
- Optional peer dependency — no impact on non-Valkey users
- Proper integration into unified memory, encoding flow, A2A cache
- Good documentation with docstrings and forward-compat notes
148515c to
3a5ecce
Compare
Contributor
|
Hey @MatthiasHowellYopp , this is nice! For digestibility, can you split this into multiple PRs? ~10k lines is too large for us to review |
3a5ecce to
04b4254
Compare
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds Valkey as a storage backend for CrewAI's unified memory system, using the valkey-glide client. Valkey is a high-performance, Redis-compatible key-value store that provides a distributed, production-ready alternative to the existing LanceDB and Qdrant storage options.
What's included
ValkeyStorage (valkey_storage.py) — full StorageBackend implementation:
CRUD operations (save, get, update, delete) with both sync and async APIs
Server-side vector search via Valkey Search module (FT.SEARCH with KNN)
Scope-based record organization with hierarchical scope queries
Category indexing and filtering
Metadata filtering with AND logic
Pagination support (limit/offset) for list_records
Scope introspection (get_scope_info, list_scopes, list_categories, count)
Bulk delete with scope, category, metadata, and age-based filters
Connection retry with exponential backoff for transient errors
Lazy client initialization and async context manager support
reset for clearing all or scoped records
ValkeyCache (valkey_cache.py) — lightweight cache interface:
get/set/delete/exists with optional TTL
JSON serialization for complex values
Connection timeout handling
Used by A2A agent card caching and file upload caching as an alternative to Redis
Integration points:
unified_memory.py — wired as a storage backend option
encoding_flow.py — encoding flow support for Valkey storage
memory_tools.py — memory tool descriptions updated with clearer parameter docs
upload_cache.py — added ValkeyCache as a cache backend option (alongside memory/Redis)
agent_card.py / task.py — added VALKEY_URL environment variable support for aiocache configuration, with priority over REDIS_URL
Optional dependency:
Added valkey = ["valkey-glide>=1.3.0"] to [project.optional-dependencies] in
pyproject.toml
Install with pip install crewai[valkey]
Tests
~5,300 lines of tests across 5 test files covering:
Core CRUD and index operations (test_valkey_storage.py)
Vector search with filters, scoring, pagination (test_valkey_storage_search.py)
Scope operations, listing, categories, count, reset (test_valkey_storage_scope.py)
Error handling, retry, serialization/deserialization (test_valkey_storage_errors.py)
Cache operations with TTL (test_valkey_cache.py)
All tests use mocked Valkey clients — no running Valkey instance required.
fixes #5578