Add support for Redis Cluster #368
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @mike182uk, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces support for Redis Cluster within the @fedify/redis package, enabling kvstore and mq components to utilize a clustered Redis setup. The change allows ioredis Cluster instances to be used as a drop-in replacement for Redis instances, maintaining the same interface and methods. This enhancement provides the benefit of allowing applications to leverage the scalability and high availability of Redis Cluster for their key-value store and message queue needs. The update includes necessary type adjustments and documentation examples to guide users on integrating clustered Redis.
Highlights
- Redis Cluster Support: The
RedisKvStoreandRedisMessageQueueclasses now accept anioredisClusterinstance in their constructors, allowing the use of Redis Cluster for key-value storage and message queuing. - Updated Documentation and Examples: The
packages/redis/README.mdfile has been updated with clear TypeScript examples demonstrating how to initializeRedisKvStoreandRedisMessageQueueusing both standalone Redis and Redis Cluster configurations. - Enhanced Type Compatibility: Type definitions for the Redis client instances within
RedisKvStoreandRedisMessageQueuehave been updated toRedis | Cluster, providing type safety for both standalone and clustered Redis connections. - TypeScript Interface Compatibility Fix: A specific TypeScript cast was introduced in
packages/redis/src/mq.tsfor theonandoffevent methods on thesubRedisclient. This addresses an incompatibility between theioredisRedisandClusterTypeScript interfaces, ensuring proper pub/sub functionality while maintaining type correctness.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
github-actions
bot
added
component/kv
There was a problem hiding this comment.
Code Review
This pull request adds support for Redis Cluster, which is a great enhancement. The changes in kv.ts and mq.ts are well-implemented, including the handling of type incompatibilities between ioredis's Redis and Cluster classes. However, I've noticed that there are no new tests to cover the Redis Cluster functionality. Adding tests for this new feature would be crucial to ensure its stability and prevent future regressions. I've also left a few minor suggestions in the code to improve documentation and readability.
|
The docs for this pull request have been published: |
There was a problem hiding this comment.
iirc, pubsub works weirdly in cluster, same with anything that needs a queue, which is why Mastodon implements redis sentinel instead — plus redis cluster is a right PITA to run/test locally, because it requires like 3 containers minimum (by contrast, kafka can work with one now)
|
Also, just having the hosts alone for cluster won't help you if you're trying to deploy against say AWS MemoryDB (all I remember from doing this last year was that getting it working was a right pain) |
|
The latest push to this pull request has been published to JSR and npm as a pre-release:
|
Summary
Added support for Redis Cluster
This change allows for
Clusterto be used instead ofRediswhen initialising thekvstore/mqClusterimplements the same interface asRedisand exposes the same methods, so should be ok as a drop-in replacementRelated Issue
@fedify/redispackage #367Changes
Redis
kvandmqupdated to acceptClusterin place ofRedisBenefits
Allows clustered Redis to be used
Checklist
deno task test-allon your machine?Additional Notes
n/a