Skip to content

Add the redis-family configuration option for ioredis to support ipv6 #532

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

kdevan
Copy link

@kdevan kdevan commented Jul 11, 2023

Let me know if I covered everything here, i'll make any changes you'd like to see or add something that I missed.

I added:

  • Configuration option to bin/redis-commander.js
  • Environment variable support to docker/entrypoint.sh
  • Updated README.md

This adds support for the [options.family] param as found here: https://ioredis.readthedocs.io/en/latest/API/

Although it says string there, type checking expects a number and it works with a number.

I needed this feature so I could deploy this project on Fly.io. Everything on Fly.io uses ipv6 internally so connecting to Redis must be done using ipv6.

@kdevan
Copy link
Author

kdevan commented Jul 11, 2023

I see this commit now: sseide@fba7e53

In my case I would still like to set the family because for the host I use a hostname instead of an IP address. But I'm updating the default that I set to 0 instead of 4 and checking out the util file.

@kdevan
Copy link
Author

kdevan commented Jul 11, 2023

Ok so it seems to me that lib/util.js sets the default configuration parameters. Then if the user provides some parameters of their own the connection object is created again in bin/redis-commander.js.

So in bin/redis-commander.js I added to the condition that checks if a new connection object should be created to include the redis-family argument. If it exists the new connection object is created using the redis-family argument, or if a different condition triggered the connection object and redis-family is not included, then it will also default to 0 there.

It seems to me this should cover everything but I'm unsure about including redis-family as part of the condition.

@kdevan
Copy link
Author

kdevan commented Jul 11, 2023

Lol well, I double checked and the value 0 is working. I must have tested this before that commit was made. You can ignore this. I'll leave it just in case but feel free to close.

@sseide
Copy link
Collaborator

sseide commented Jul 27, 2023

thanks for the PR. but as you already found out latest version was working :-)
Nonetheless i think it might be handy for someone to force IPv4 or IPv6 with such an option... What do you think?

@kdevan
Copy link
Author

kdevan commented Jul 28, 2023

Maybe wouldn't hurt. But I can't think of any use cases off hand. I need to use ipv4 for development and ipv6 for stage/production and the 0 value does work for both so far. I'm just getting some of this up on stage now though so I'll add a comment here if I run into any scenario where specifying ipv6 ends up mattering.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants