Conversation
Pre-configured factory methods that declare caching intent rather than manually composing backends, L1, and encryption: CacheKit::minimal(url).await? — Redis, no L1, 300s TTL CacheKit::production(url).await? — Redis + L1, 600s TTL CacheKit::encrypted(url, key).await? — Redis + L1 + AES-256-GCM, 600s TTL CacheKit::io(api_key)? — SaaS + L1, 3600s TTL Each returns a CacheKitBuilder so callers can override any default. Redis intents are async (eager connect via fred). io() is sync (HTTP). Named `encrypted` instead of `secure` to avoid collision with the existing CacheKit::secure() method that returns SecureCache. Closes #13
📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant CK as CacheKit::intent
participant Builder as CacheKitBuilder
participant Backend as BackendFactory
participant Redis as Redis (external)
participant IO as cachekit.io (external)
rect rgba(200,230,255,0.5)
Dev->>CK: call CacheKit::production(redis_url)
CK->>Backend: construct Redis backend (platform wrapper)
Backend->>Redis: connect() (eager)
Redis-->>Backend: connection result / error
Backend-->>Builder: return configured builder (TTL=600, L1 cap=1000)
Builder-->>Dev: Result<CacheKitBuilder, CachekitError>
end
sequenceDiagram
participant Dev as Developer
participant CK as CacheKit::encrypted
participant Builder as CacheKitBuilder
participant Backend as BackendFactory
participant Redis as Redis (external)
participant Crypto as encryption_from_bytes
rect rgba(230,245,200,0.5)
Dev->>CK: call CacheKit::encrypted(redis_url, master_key)
CK->>Crypto: validate/derive encryption using master_key
Crypto-->>CK: encryption config / error
CK->>Backend: construct Redis backend
Backend->>Redis: connect() (eager)
Redis-->>Backend: connection result / error
Backend-->>Builder: configured builder (TTL=600, L1 cap=1000, encryption)
Builder-->>Dev: Result<CacheKitBuilder, CachekitError>
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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/cachekit/src/lib.rs`:
- Around line 38-39: Update the top-level module doc to match the new intent API
name: replace the outdated reference to CacheKit::secure with
CacheKit::encrypted in the module comment that lists intent-based cache presets
(the line mentioning `CacheKit::minimal`, `::production`, `::secure`, `::io`).
Verify the doc string near the `mod intents;` declaration references the actual
constructor names implemented (e.g., `CacheKit::encrypted`) so the documentation
accurately reflects the API.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: dd51bfc5-8851-407f-a72f-f28ee587bb77
📒 Files selected for processing (3)
crates/cachekit/src/intents.rscrates/cachekit/src/lib.rscrates/cachekit/tests/intent_tests.rs
- Fix doc comment in lib.rs: ::secure → ::encrypted - Fix master key doc: "at least 16 bytes" → "at least 32 bytes" - DRY: replace 4 cfg-gated wrap_redis/wrap_cachekitio fns with single generic wrap() (2 cfg variants instead of 4)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/cachekit/src/intents.rs (1)
60-69: Extract shared Redis eager-connect setup to avoid drift.The Redis bootstrap block is duplicated three times. A small helper will keep behavior aligned and reduce maintenance risk.
♻️ Proposed refactor
+#[cfg(feature = "redis")] +async fn connected_redis_backend(redis_url: &str) -> Result<SharedBackend, CachekitError> { + let backend = crate::backend::redis::RedisBackend::builder() + .url(redis_url) + .build()?; + backend.connect().await?; + Ok(wrap(backend)) +} + #[cfg(feature = "redis")] pub async fn minimal(redis_url: &str) -> Result<CacheKitBuilder, CachekitError> { - let backend = crate::backend::redis::RedisBackend::builder() - .url(redis_url) - .build()?; - drop(backend.connect().await?); + let backend = connected_redis_backend(redis_url).await?; Ok(CacheKitBuilder::default() - .backend(wrap(backend)) + .backend(backend) .default_ttl(Duration::from_secs(300)) .no_l1()) } #[cfg(feature = "redis")] pub async fn production(redis_url: &str) -> Result<CacheKitBuilder, CachekitError> { - let backend = crate::backend::redis::RedisBackend::builder() - .url(redis_url) - .build()?; - drop(backend.connect().await?); + let backend = connected_redis_backend(redis_url).await?; Ok(CacheKitBuilder::default() - .backend(wrap(backend)) + .backend(backend) .default_ttl(Duration::from_secs(600)) .l1_capacity(1000)) } #[cfg(all(feature = "redis", feature = "encryption"))] pub async fn encrypted( redis_url: &str, master_key: &[u8], ) -> Result<CacheKitBuilder, CachekitError> { - let backend = crate::backend::redis::RedisBackend::builder() - .url(redis_url) - .build()?; - drop(backend.connect().await?); + let backend = connected_redis_backend(redis_url).await?; CacheKitBuilder::default() - .backend(wrap(backend)) + .backend(backend) .default_ttl(Duration::from_secs(600)) .l1_capacity(1000) .encryption_from_bytes(master_key, "default") }Also applies to: 96-105, 141-151
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/cachekit/src/intents.rs` around lines 60 - 69, Extract the duplicated Redis bootstrap into a single async helper (e.g., async fn build_eager_redis_backend(redis_url: &str) -> Result<crate::backend::redis::RedisBackend, E>) that encapsulates RedisBackend::builder().url(redis_url).build()? followed by backend.connect().await? (drop the connection) and returns the built backend; then replace each duplicated block that calls RedisBackend::builder() / build() / connect().await? in intents.rs (including the spots currently using wrap(backend)) with a call to this helper and preserve existing error propagation and wrapping (wrap(backend)); keep the helper signature async and returning the same error type so callers can use ? without changing call sites.
🤖 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/cachekit/src/intents.rs`:
- Around line 60-63: The code currently constructs Redis backends with the same
pattern (RedisBackend::builder().url(...).build() and an eager connect().await)
which fails to encode intent-specific resilience (minimal vs
production/encrypted); update each construction site to apply intent-specific
configuration instead of identical builds: use
RedisBackend::builder().url(...).retry_policy(...) / .circuit_breaker(...) for
production, add TLS options for encrypted (e.g., .tls_enabled(true) or
equivalent), and use a lightweight health check or deferred connect for minimal
(avoid forcing connect().await); remove or replace the unconditional
drop(connect().await?) calls and ensure the builder methods (retry_policy,
circuit_breaker, tls_enabled or similar) are used on the RedisBackend::builder()
invocations to encode semantics for minimal/production/encrypted intents.
---
Nitpick comments:
In `@crates/cachekit/src/intents.rs`:
- Around line 60-69: Extract the duplicated Redis bootstrap into a single async
helper (e.g., async fn build_eager_redis_backend(redis_url: &str) ->
Result<crate::backend::redis::RedisBackend, E>) that encapsulates
RedisBackend::builder().url(redis_url).build()? followed by
backend.connect().await? (drop the connection) and returns the built backend;
then replace each duplicated block that calls RedisBackend::builder() / build()
/ connect().await? in intents.rs (including the spots currently using
wrap(backend)) with a call to this helper and preserve existing error
propagation and wrapping (wrap(backend)); keep the helper signature async and
returning the same error type so callers can use ? without changing call sites.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f4586e92-1b1a-4b65-9ee8-e2db7cd62f7b
📒 Files selected for processing (2)
crates/cachekit/src/intents.rscrates/cachekit/src/lib.rs
✅ Files skipped from review due to trivial changes (1)
- crates/cachekit/src/lib.rs
Comment on lines
+60
to
+63
| let backend = crate::backend::redis::RedisBackend::builder() | ||
| .url(redis_url) | ||
| .build()?; | ||
| drop(backend.connect().await?); |
There was a problem hiding this comment.
Intent resilience behavior is not differentiated.
Line 60, Line 96, and Line 141 all construct Redis the same way (builder().url().build() + eager connect). This does not encode the required intent differences for circuit-breaker/retry behavior, so minimal vs production/encrypted cannot reliably match the stated objective semantics.
Also applies to: 96-99, 141-144
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/cachekit/src/intents.rs` around lines 60 - 63, The code currently
constructs Redis backends with the same pattern
(RedisBackend::builder().url(...).build() and an eager connect().await) which
fails to encode intent-specific resilience (minimal vs production/encrypted);
update each construction site to apply intent-specific configuration instead of
identical builds: use RedisBackend::builder().url(...).retry_policy(...) /
.circuit_breaker(...) for production, add TLS options for encrypted (e.g.,
.tls_enabled(true) or equivalent), and use a lightweight health check or
deferred connect for minimal (avoid forcing connect().await); remove or replace
the unconditional drop(connect().await?) calls and ensure the builder methods
(retry_policy, circuit_breaker, tls_enabled or similar) are used on the
RedisBackend::builder() invocations to encode semantics for
minimal/production/encrypted intents.
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
1 participant
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.
Summary
CacheKit::minimal(),::production(),::encrypted(),::io()factory methodsCacheKitBuilderso callers can override any defaultIntent Configuration
minimalproductionencryptedioDesign decisions
async(eager connect via fred);io()is sync (HTTP)encryptednotsecure—CacheKit::secure()already exists (returnsSecureCache)redisfor Redis intents,cachekitiofor io,encryptionfor encryptedlocal()intent — Python's value comes from skipping serialization (stores raw objects); Rust can't replicate without a different API. Use moka directly.Test plan
--features redis,--features cachekitioCloses #13
Summary by CodeRabbit