cacheable - fix: ttl in documentation and cascading ttl in set and setMany#1620
cacheable - fix: ttl in documentation and cascading ttl in set and setMany#1620
Conversation
The `set()` and `setManyKeyv()` methods now correctly follow the
documented TTL priority: explicit function TTL >> store adapter TTL >>
cacheable TTL. Previously, both primary and secondary stores received
the same TTL (cacheable default), ignoring any TTL configured on
individual Keyv store instances.
Also fixes README examples that incorrectly passed `{ ttl }` to
KeyvRedis (which doesn't accept it) — the correct approach is to
wrap KeyvRedis in a Keyv instance with the ttl option.
https://claude.ai/code/session_015CcQfg62CLeeE43HusGV57
Replace inlined TTL priority logic with the existing getCascadingTtl utility from @cacheable/utils in both set() and setManyKeyv(). https://claude.ai/code/session_015CcQfg62CLeeE43HusGV57
There was a problem hiding this comment.
Code Review
This pull request implements cascading TTL logic, allowing primary and secondary stores to utilize their own TTL configurations when an explicit TTL is not provided. The changes include updates to the set and setMany methods, documentation improvements in the README, and new test cases to verify per-store TTL behavior. A significant issue was identified in the set method where the BEFORE_SET hook's modifications to the TTL are ignored when calculating the TTL for the secondary store, potentially causing data inconsistency between layers.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 259a43236d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
When the BEFORE_SET hook modifies item.ttl, that override now applies to the secondary store as well. Previously the secondary TTL was computed independently, ignoring the hook's modification. https://claude.ai/code/session_015CcQfg62CLeeE43HusGV57
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1620 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 26 26
Lines 2492 2496 +4
Branches 556 555 -1
=========================================
+ Hits 2492 2496 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
The
set()andsetManyKeyv()methods now correctly follow thedocumented TTL priority: explicit function TTL >> store adapter TTL >>
cacheable TTL. Previously, both primary and secondary stores received
the same TTL (cacheable default), ignoring any TTL configured on
individual Keyv store instances.
Also fixes README examples that incorrectly passed
{ ttl }toKeyvRedis (which doesn't accept it) — the correct approach is to
wrap KeyvRedis in a Keyv instance with the ttl option.
https://claude.ai/code/session_015CcQfg62CLeeE43HusGV57