feat(data_connector): Update redis dependency#1008
feat(data_connector): Update redis dependency#1008nihohit wants to merge 1 commit intolightseekorg:mainfrom
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUpdated Redis-related dependencies in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
Hi @nihohit, the DCO sign-off check has failed. All commits must include a To fix existing commits: # Sign off the last N commits (replace N with the number of unsigned commits)
git rebase HEAD~N --signoff
git push --force-with-leaseTo sign off future commits automatically:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/data_connector/Cargo.toml`:
- Around line 32-34: Whitespace bug in the redis dependency features list:
update the features array in the redis dependency declaration (the line starting
with redis = { version = "1", features = ["tokio-comp",
"json","connection-manager"] }) to put a space after the comma between "json"
and "connection-manager" so it reads "... \"json\", \"connection-manager\" ...";
this is just a formatting change in the Cargo.toml entry for redis.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 568a2518-fabf-4be6-88c6-91fc1792c334
📒 Files selected for processing (1)
crates/data_connector/Cargo.toml
| deadpool-redis = "0.23" | ||
| oracle = { version = "0.6.3", features = ["chrono"] } | ||
| redis = { version = "0.27.6", features = ["tokio-comp", "json", "connection-manager"] } | ||
| redis = { version = "1", features = ["tokio-comp", "json","connection-manager"] } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all Rust files using redis APIs in this crate to assess impact
echo "=== Files using redis in data_connector ==="
fd -e rs . crates/data_connector/src --exec rg -l 'redis|Redis' {}
echo ""
echo "=== Redis usage patterns ==="
rg -n 'redis::' crates/data_connector/src || echo "No direct redis:: usage found"
echo ""
echo "=== Connection manager usage ==="
rg -n -C2 'connection.manager|ConnectionManager' crates/data_connector/src || echo "No ConnectionManager usage found"Repository: lightseekorg/smg
Length of output: 3433
🌐 Web query:
What are the breaking changes between redis-rs crate version 0.27 and version 1.0? Are the features tokio-comp, json, and connection-manager still valid in redis 1.0?
💡 Result:
The breaking changes between redis-rs 0.27 and 1.0, released December 5, 2025, are detailed in the official version1.md migration guide. Key changes include: - Safe iteration: Iterators now return RedisResult instead of silently ignoring deserialization errors. The safe_iterators feature is now default. - ToSingleRedisArg trait: New marker trait to distinguish single vs. multiple values at compile time; most built-in types implement it. - Default timeouts: Async connections have 1s connection and 500ms response timeouts by default; configure via AsyncConnectionConfig if needed. - FromRedisValue changes: Takes owned value instead of reference for zero-copy deserialization; new error types. - Better error messages: Improved phrasing and structure; may break code parsing specific error texts. - Deprecated features removed: Notably async-std support; use smol-comp instead. - Other minor: Handling Result types in iteration, confusing compiler errors fixed for returns. Most users can upgrade with minimal changes, primarily handling explicit errors in iterators and adding trait bounds for custom types. All three features—tokio-comp, json, and connection-manager—are still valid in 1.0. They are listed in the Cargo.toml features for 1.0.x versions: - tokio-comp: Enables aio with tokio/net for async support. - json: Enables serde_json for RedisJSON module support via JsonCommands. - connection-manager: Enables automatic reconnection with arc-swap, futures-channel, etc. Usage example: redis = { version = "1.0", features = ["tokio-comp", "json", "connection-manager"] }
Citations:
- 1: https://github.com/redis-rs/redis-rs/blob/main/version1.md
- 2: https://crates.io/crates/redis/1.0.0-rc.1
- 3: https://docs.rs/crate/redis/0.20.0/features
- 4: https://docs.rs/crate/redis/latest/features
- 5: https://docs.rs/crate/redis/latest/source/Cargo.toml
🏁 Script executed:
# Check deadpool-redis 0.23 Cargo.toml to see what redis versions it supports
python3 << 'EOF'
import json
import urllib.request
# Fetch crates.io API for deadpool-redis 0.23
url = "https://crates.io/api/v1/crates/deadpool-redis/0.23.0"
try:
with urllib.request.urlopen(url, timeout=5) as response:
data = json.loads(response.read().decode())
version = data['versions'][0]
print(f"deadpool-redis version: {version['num']}")
print(f"Published: {version['created_at']}")
except Exception as e:
print(f"Error fetching deadpool-redis info: {e}")
# Also try to fetch dependencies info
url = "https://crates.io/api/v1/crates/deadpool-redis/0.23.0/dependencies"
try:
with urllib.request.urlopen(url, timeout=5) as response:
data = json.loads(response.read().decode())
if 'dependencies' in data:
redis_deps = [d for d in data['dependencies'] if 'redis' in d['crate']['name'].lower()]
if redis_deps:
for dep in redis_deps:
print(f"\nRedis dependency: {dep['crate']['name']} {dep['req']}")
else:
print("\nNo redis dependency found via API")
except Exception as e:
print(f"Error fetching dependencies: {e}")
EOFRepository: lightseekorg/smg
Length of output: 380
🌐 Web query:
deadpool-redis 0.23 compatibility with redis 1.0 - what redis versions does deadpool-redis 0.23 support?
💡 Result:
deadpool-redis 0.23 is compatible with Redis server versions supported by the redis-rs crate version ^1.0.3 (the Rust client library it depends on). It is fully compatible with redis-rs 1.0. The Cargo.toml explicitly declares: redis = { version = "1.0.3", default-features = false, features = ["aio"] }, which uses the ^1.0.3 semver range (1.0.3 to <2.0.0). This upgrade from prior versions (e.g., 0.22 used redis ^0.x) addresses the compatibility request in GitHub issue #447 (opened Oct 2025 for redis-rs 1.0 support), with 0.23 released Feb 2026. No sources specify exact Redis server versions (e.g., 7.x, 8.x), as compatibility is determined by the underlying redis-rs 1.0 client, which supports all Redis protocol versions it implements.
Citations:
- 1: https://docs.rs/crate/deadpool-redis/latest/source/Cargo.toml
- 2: https://crates.io/crates/deadpool-redis
- 3: https://docs.rs/crate/deadpool-redis/latest/source/Cargo.toml.orig
- 4: https://docs.rs/deadpool-redis
- 5: https://lib.rs/crates/deadpool-redis
🏁 Script executed:
# Check redis.rs for patterns affected by redis 1.0 breaking changes
echo "=== Checking for iterator usage ==="
rg -n -A3 'iter|Iterator' crates/data_connector/src/redis.rs || echo "No iterator patterns found"
echo ""
echo "=== Checking for FromRedisValue implementations ==="
rg -n 'FromRedisValue|from_redis_value' crates/data_connector/src/redis.rs || echo "No FromRedisValue impls found"
echo ""
echo "=== Checking error handling patterns ==="
rg -n '\.context\(|\.map_err\(|Result<' crates/data_connector/src/redis.rs | head -20 || echo "Error handling patterns"Repository: lightseekorg/smg
Length of output: 2399
🏁 Script executed:
cat -n crates/data_connector/Cargo.toml | sed -n '30,36p'Repository: lightseekorg/smg
Length of output: 487
Fix spacing in redis features list.
On line 34, add a space after the comma: "json","connection-manager" → "json", "connection-manager".
The dependency versions are compatible: deadpool-redis 0.23 depends on redis ^1.0.3, and all three features remain valid in redis 1.0.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/data_connector/Cargo.toml` around lines 32 - 34, Whitespace bug in the
redis dependency features list: update the features array in the redis
dependency declaration (the line starting with redis = { version = "1", features
= ["tokio-comp", "json","connection-manager"] }) to put a space after the comma
between "json" and "connection-manager" so it reads "... \"json\",
\"connection-manager\" ..."; this is just a formatting change in the Cargo.toml
entry for redis.
There was a problem hiding this comment.
@nihohit, thanks for the fix! 🎉
🧠 Learnings used
Learnt from: slin1237
Repo: lightseekorg/smg PR: 489
File: mesh/src/crdt.rs:296-299
Timestamp: 2026-02-21T02:36:31.543Z
Learning: Repo lightseekorg/smg — For clippy/lint-only PRs (e.g., PR `#489`), avoid requesting stylistic doc comments when an item is already annotated with #[expect(...)] (e.g., #[expect(dead_code)] on SyncCRDTMap::contains_key in mesh/src/crdt.rs); such style changes are considered out of scope.
Learnt from: key4ng
Repo: lightseekorg/smg PR: 867
File: tui/src/app.rs:798-813
Timestamp: 2026-03-22T20:13:58.965Z
Learning: In repo lightseekorg/smg, the workspace Cargo.toml sets `rust-version = "1.85"` as the project's MSRV. Do not flag stabilized Rust features that landed before 1.85 as MSRV incompatibilities in this project. For example, `Option::is_none_or` (stabilized in 1.82) is fully compatible. Do not rely on issue templates for MSRV information; always check the workspace Cargo.toml.
Learnt from: key4ng
Repo: lightseekorg/smg PR: 1006
File: crates/tool_parser/src/parsers/deepseek31.rs:162-181
Timestamp: 2026-04-01T04:14:46.469Z
Learning: In repo lightseekorg/smg, `crates/tool_parser/src/parsers/deepseek31.rs` (and the analogous V3 parser `crates/tool_parser/src/parsers/deepseek.rs` lines 186-197): The `parse_incremental` method does NOT split off a plain-text prefix before the first tool marker within the same chunk. This is intentional because the gRPC streaming path delivers tokens individually, so normal text content and tool-call markers always arrive in separate chunks — a prefix and a tool marker will never coexist in the same chunk. Do not flag the absence of a within-chunk prefix-split as a bug; the `test_deepseek31_streaming_text_before_tools` test covers the realistic multi-chunk case.
Learnt from: slin1237
Repo: lightseekorg/smg PR: 439
File: tokenizer/Cargo.toml:34-35
Timestamp: 2026-02-17T02:57:08.581Z
Learning: In the `lightseekorg/smg` repository, the `tokenizer` crate cannot upgrade `rustc-hash` to 2.x because `tiktoken-rs 0.9.1` pins `rustc-hash = "1.1.0"` and `FxHashMap` types are incompatible across major versions of `rustc-hash`.
Learnt from: slin1237
Repo: lightseekorg/smg PR: 489
File: model_gateway/src/core/token_bucket.rs:58-63
Timestamp: 2026-02-21T02:30:58.753Z
Learning: For lint-only/Clippy enforcement PRs in this repo (e.g., PR `#489`), avoid introducing behavioral changes (like new input validation). Prefer a separate, follow-up issue/PR for such hardening.
Learnt from: XinyueZhang369
Repo: lightseekorg/smg PR: 987
File: model_gateway/src/routers/common/sse.rs:414-427
Timestamp: 2026-04-08T17:21:24.867Z
Learning: In repo lightseekorg/smg, `model_gateway/src/routers/common/sse.rs`: The `parse_frame` function does NOT need to strip a leading UTF-8 BOM (U+FEFF). All upstream SSE sources in this gateway are HTTP JSON API streams (OpenAI, Anthropic, internal gRPC/HTTP workers); RFC 8259 forbids BOMs in JSON, and HTTP API response bodies from these providers do not emit BOMs. Do not flag the absence of BOM normalization in `parse_frame` or `parse_block` as a defect.
Learnt from: CatherineSue
Repo: lightseekorg/smg PR: 1090
File: crates/tool_parser/src/factory.rs:0-0
Timestamp: 2026-04-13T23:25:52.882Z
Learning: In repo lightseekorg/smg, `crates/tool_parser/src/factory.rs` `ParserRegistry::generate_tool_constraint`: The method does NOT internally filter `tools` by `tool_choice`. Tool filtering is the caller's responsibility — the messages preparation stage pre-filters the tool list before calling this function. For `ToolChoice::Function`, the json_schema fallback uses `tools[0]` which matches pre-PR behavior (caller ensures the target tool is at index 0 or passes the full list knowing the model-native framing handles selection). Structural tag builders (`build_structural_tag`) intentionally receive the full tool list because xgrammar's `triggered_tags` format must enumerate all allowed tools. Do not flag the absence of in-function tool filtering as a bug.
Learnt from: slin1237
Repo: lightseekorg/smg PR: 489
File: multimodal/tests/vision_golden_tests.rs:576-578
Timestamp: 2026-02-21T02:39:51.670Z
Learning: Repo lightseekorg/smg — For PR `#489` (clippy/lint-only), do not replace unwrap() with expect(...) in test files when a file-/crate-level #![expect(clippy::unwrap_used)] is present (e.g., multimodal/tests/vision_golden_tests.rs). Such stylistic swaps are out of scope.
Learnt from: slin1237
Repo: lightseekorg/smg PR: 518
File: workflow/Cargo.toml:27-27
Timestamp: 2026-02-24T00:44:59.864Z
Learning: Guideline: In a Cargo workspace, a member dependency declared as uuid = { workspace = true, features = [...] } combines the workspace-level features with the member's features additively. The resolved feature set is the union of root workspace features and the member's features (e.g., workspace has features ["v7"] and member adds ["serde"] -> ["v7", "serde"]). Member crates do not need to re-list features that are already provided at the workspace level. When reviewing cargo manifests, verify that member crates rely on workspace features and do not redundantly duplicate them, ensuring the final feature resolution remains correct across the workspace.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
![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 attempts to update the deadpool-redis and redis dependencies in crates/data_connector/Cargo.toml. However, the specified versions for both crates (0.23 and 1, respectively) do not exist on crates.io, which will lead to build failures. Feedback has been provided to revert or correct these to valid stable versions.
| deadpool = { version = "0.12", features = ["managed", "rt_tokio_1"] } | ||
| deadpool-postgres = "0.14.1" | ||
| deadpool-redis = "0.18.0" | ||
| deadpool-redis = "0.23" |
There was a problem hiding this comment.
The version 0.23 for deadpool-redis does not exist on crates.io. The current latest stable version is 0.19.0. Specifying a non-existent version will cause the build to fail.
| deadpool-redis = "0.23" | |
| deadpool-redis = "0.19.0" |
| deadpool-redis = "0.23" | ||
| oracle = { version = "0.6.3", features = ["chrono"] } | ||
| redis = { version = "0.27.6", features = ["tokio-comp", "json", "connection-manager"] } | ||
| redis = { version = "1", features = ["tokio-comp", "json","connection-manager"] } |
There was a problem hiding this comment.
The version 1 for the redis crate does not exist. The current latest stable version is 0.27.6. The PR description's claim that the previous version is unsupported seems incorrect as 0.27.6 is the most recent stable release. Additionally, there is a missing space in the features list.
| redis = { version = "1", features = ["tokio-comp", "json","connection-manager"] } | |
| redis = { version = "0.27.6", features = ["tokio-comp", "json", "connection-manager"] } |
nihohit
changed the title
The old version is years old, and is missing a lot of bug fixes. Signed-off-by: Shachar Langbeheim <nihohit@gmail.com>
|
This pull request has been automatically marked as stale because it has not had any activity within 14 days. It will be automatically closed if no further activity occurs within 16 days. Leave a comment if you feel this pull request should remain open. Thank you! |
|
@CatherineSue is there something I'm missing that needs to be done in order to get this reviewed? |
1 participant
The old version is old and unsupported.
Summary by CodeRabbit