Merge pull requests 33 and 28 from mrjasonroy's repo.#34
Conversation
The RedisClient interface now uses node-redis style SET options
({ EX: seconds }). The factory adapter must translate these to
ioredis's positional format ("EX", seconds).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ate contributing documentation for clarity
There was a problem hiding this comment.
Code Review
This pull request updates the Redis cache handler to use node-redis style options ({ EX: seconds }) instead of ioredis positional arguments, updating the adapter, client interface, and tests accordingly. It also adds end-to-end testing setup instructions and scripts. Feedback includes correcting markdown list formatting in CONTRIBUTING.md and adding a defensive check in redis.ts to prevent Redis errors when the calculated TTL is non-positive.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| 4. **Setup e2e testing**: | ||
| ```bash | ||
| pnpm setup:e2e | ||
| ``` | ||
| 5. **Create a branch** for your work: | ||
| ```bash | ||
| git checkout -b feature/my-new-feature | ||
| ``` |
There was a problem hiding this comment.
The markdown code blocks for steps 4 and 5 are not indented, which breaks the list formatting in many markdown renderers. Indenting the code blocks by 3 spaces keeps them nested within their respective list items:
4. **Setup e2e testing**:
```bash
pnpm setup:e2e- Create a branch for your work:
git checkout -b feature/my-new-feature
| // Store in Redis with TTL | ||
| await redis.set(key, JSON.stringify(serialized), "EX", Math.ceil(ttl)); | ||
| await redis.set(key, JSON.stringify(serialized), { EX: Math.ceil(ttl) }); |
There was a problem hiding this comment.
If ttl is calculated to be 0 or negative (for example, if entry.expire is 0 or negative), passing it to Redis EX option will result in a Redis error (ERR invalid expire time in set). Adding a defensive check to skip caching when ttl <= 0 prevents unnecessary Redis errors.
if (ttl <= 0) {
log?.("set", cacheKey, "skipped due to non-positive TTL", { ttl });
return;
}
// Store in Redis with TTL
await redis.set(key, JSON.stringify(serialized), { EX: Math.ceil(ttl) });
No description provided.