Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: replace redis library with ioredis #1240

Merged
merged 3 commits into from
Mar 29, 2022

Conversation

BelgianNoise
Copy link
Member

📁 Related issues

/

✍️ Description

The redis resource locker currently does not support redis clusters. The current redis library used is too "dumb" to handle clusters appropriatly (extra info on this page).

ioredis has built in cluster support while redis does not.

@BelgianNoise BelgianNoise added the ☀️ enhancement New feature or request label Mar 24, 2022
@BelgianNoise BelgianNoise self-assigned this Mar 24, 2022
@BelgianNoise BelgianNoise marked this pull request as ready for review March 25, 2022 07:43
@@ -1,6 +1,6 @@
import { assert } from 'console';
import type { RedisClient } from 'redis';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joachimvh I think we should ignore the warning below; up to you whether locally or globally.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I was planning to look into this to see if we need to change the linter settings or the import later. Was actually surprised our linter could throw warnings so also going to change that so it's always errors.

@joachimvh
Copy link
Member

@BelgianNoise was having a look and noticed IORedis released v5.x 2 days ago. Would it be interesting to go for that version then?

@BelgianNoise
Copy link
Member Author

@BelgianNoise was having a look and noticed IORedis released v5.x 2 days ago. Would it be interesting to go for that version then?

I checked out the change logs and there dont seem to be any major changes, bug fixes and stability improvements mainly. I ill bump the version tomorrow.

@BelgianNoise
Copy link
Member Author

@BelgianNoise was having a look and noticed IORedis released v5.x 2 days ago. Would it be interesting to go for that version then?

I checked out the change logs and there dont seem to be any major changes, bug fixes and stability improvements mainly. I ill bump the version tomorrow.

@joachimvh, I tried updateing ioredis to 5.0.1 but there seems to be a change in how typings work which do not make them compatible with redlock. redlock is however being updated but ioredis 5.x.x support is still in beta.
Won't be able to bump at this moment.

@joachimvh
Copy link
Member

Let's stay with this version then for now. I'll look into the linter warning.

@joachimvh joachimvh merged commit 5d802c6 into versions/4.0.0 Mar 29, 2022
@joachimvh joachimvh deleted the chore/replace-redis-library-by-ioredis branch March 29, 2022 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
☀️ enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants