search

joeferner / redis-commander

Redis management tool written in node.js
http://joeferner.github.io/redis-commander/
MIT License
3.61k stars 467 forks source link

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

Open kdevan opened 1 year ago

kdevan commented 1 year ago

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:

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 commented 1 year ago

I see this commit now: https://github.com/sseide/redis-commander/commit/fba7e5305ebdf437b98ff34e42f34ed34d939edc

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.