Introduce cache mechanism support#160
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR introduces a complete request-scoped HTTP response caching system for FrameX with in-memory and file-based storage backends, configuration validation, request-header-based cache control (USE/BYPASS/REFRESH), and integration into route handlers and decorators. ChangesRequest-level HTTP response caching
Sequence DiagramsequenceDiagram
participant Client
participant RouteHandler
participant RequestCache
participant MemoryStore
participant Adapter
Client->>RouteHandler: GET /api/data?x=1
RouteHandler->>RouteHandler: check cache enabled
RouteHandler->>RequestCache: call(request, invoke)
RequestCache->>RequestCache: parse X-FrameX-Cache header
RequestCache->>RequestCache: build cache key
RequestCache->>MemoryStore: get(hashed_key)
alt First request (MISS)
MemoryStore-->>RequestCache: null
RequestCache->>Adapter: invoke()
Adapter-->>RequestCache: {data: value}
RequestCache->>MemoryStore: set(hashed_key, {data: value})
RequestCache->>RouteHandler: {data: value} + X-FrameX-Cache-Status: MISS
else Subsequent request (HIT)
MemoryStore-->>RequestCache: {data: value}
RequestCache->>RouteHandler: {data: value} + X-FrameX-Cache-Status: HIT
end
RouteHandler->>Client: response + cache headers
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/framex/driver/cache.py`:
- Around line 295-296: In _stable_value, the current branch treats
list/tuple/set the same, but iterating a set is non-deterministic; update the
set handling in the _stable_value function so that when value is a set you first
convert each item via _stable_value, then sort the resulting items
deterministically (for example by their string/JSON representation or a stable
key) and return that sorted list so equivalent sets produce the same cache key.
- Around line 89-120: When _cache_action(request) returns CacheAction.BYPASS you
must skip both cache reads and cache writes: detect CacheAction.BYPASS (in
addition to the existing CacheAction.USE branch), call and return await invoke()
immediately, set response.headers[CACHE_STATUS_HEADER] = CacheStatus.BYPASS, and
do not call self.set or create CacheEntryMetadata for that request; keep
existing behavior for CacheAction.USE (attempt get) and CacheAction.REFRESH
(allow write/refresh).
In `@tests/driver/test_request_cache.py`:
- Around line 175-177: In the test's custom key-builder (function build_key)
you're appending a dict-keys view to seen_keys which causes equality checks to
fail; change the capture to a concrete list by replacing
seen_keys.append(context.keys()) with seen_keys.append(list(context.keys()))
(and make the same list(...) conversion for any other places that append or
record context.keys() so the later assertions that compare against concrete
lists (related to seen_keys) will succeed); refer to build_key and seen_keys
when making the change.
🪄 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: CHILL
Plan: Pro
Run ID: a59dcfa7-82c8-4e6d-9055-b70213fc27fd
📒 Files selected for processing (8)
src/framex/config.pysrc/framex/consts.pysrc/framex/driver/cache.pysrc/framex/driver/ingress.pysrc/framex/plugin/on.pytests/driver/test_request_cache.pytests/test_config.pytests/test_plugin.py
| def build_key(request: Request, context: Any) -> str: | ||
| seen_keys.append(context.keys()) | ||
| return request.url.path |
There was a problem hiding this comment.
Fix key list capture type in custom key-builder test
Line 176 appends a keys-view, but Line 202 asserts concrete lists; this can fail the assertion despite correct cache behavior.
Proposed fix
def build_key(request: Request, context: Any) -> str:
- seen_keys.append(context.keys())
+ seen_keys.append(list(context.keys()))
return request.url.pathAlso applies to: 202-203
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/driver/test_request_cache.py` around lines 175 - 177, In the test's
custom key-builder (function build_key) you're appending a dict-keys view to
seen_keys which causes equality checks to fail; change the capture to a concrete
list by replacing seen_keys.append(context.keys()) with
seen_keys.append(list(context.keys())) (and make the same list(...) conversion
for any other places that append or record context.keys() so the later
assertions that compare against concrete lists (related to seen_keys) will
succeed); refer to build_key and seen_keys when making the change.
1 participant
Summary by CodeRabbit
New Features
Tests