search

eleme / corvus

A fast and lightweight Redis Cluster Proxy for Redis 3.0
MIT License
789 stars 143 forks source link

Make bind parameter take the actual interface, and port the port #121

Open tevino opened 7 years ago

tevino commented 7 years ago

@badboy corvus.conf should be changed as well, it's used for testing, thus the failure.

badboy commented 7 years ago

Changed corvus.conf. Keep in mind this would be a breaking change in corvus and would affect all users. I grepped through the code to find all places of config.bind usage, but I might have missed some.

badboy commented 7 years ago

In addition: this includes the minimal "get ipv6 working" patch. I did a manual test, but it would need to be tested more before getting merged.

badboy commented 7 years ago

I fixed the test now, but it's now lacking any tests for the new bind option. I will add that later.

tevino commented 7 years ago

For our production case, we get no benefits but rather extra work(config file compatibility) by merging this, but I think this is the right thing to do, so we'll merge this later on, next release I hope.

You could make changes until then.

badboy commented 7 years ago

I do understand the friction it will cause due to config file changes. I'd like to test this a bit further, including proper tests and IPv6 support. Happy to work on this for the next release

CLAassistant commented 1 year ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

badboy commented 1 year ago

lol nope.