Accept string keys in client config (sentinel)

[maximerety]

There's already code to convert string keys into symbols in the options hash, but this is not working on the "sentinel part" of the options hash.

This PR adds support to init a Redis client like this:

Redis.new({
  "url" => "redis://master1",
  "sentinels" => [
    {"host" => "127.0.0.1", "port" => 26381},
    {"host" => "127.0.0.1", "port" => 26382}
  ],
  "role" => "master"
})

whereas previously it was mandatory to use :sentinels, :host, :port and :role.

This is especially useful when initializing a Redis client from a json config file, e.g.:

Redis.new(JSON.parse(File.read("config/redis.json")))

Side notes :

• I'm also duplicating the nested array (map) and hashes (dup) of the options[:sentinels] value, to avoid modifying the original options hash (it was done here for the parent options hash)
• I'm always deleting string keys when converting them into symbols to avoid keeping "duplicate" keys (one string + one symbol)
• The test/sentinel_test.rb has been updated to test both cases (string keys vs. symbol keys)

[badboy]

Thanks. Looking into it.

[djanowski]

I'm hesitant.

In the future we'll move to named arguments, so I don't want to add more code to deal with string keys.

Especially when in your case it's just:

Redis.new(JSON.parse(File.read("config/redis.json"), symbolize_names: true))

[maximerety]

Good point!

Though even with keyword arguments, it could still not be clear if the sentinels hashes should contain symbol or string keys (the sentinels: keyword argument will not impose any limitation/check on :host vs. "host", a.s.o.). Maybe add a warning in README.md to explain that all config keys should always be symbols?

I was surprised that it worked well when testing without sentinels (e.g. Redis.new({ "host" => "localhost", "port" => 6380 }) is ok) but then it didn't work with sentinels. I had to dive into the gem code to understand the reason of this failure... which was not straightforward. And I was even more confused to see a piece of code already in charge of converting string keys to symbols.

But I won't be offended if you decide this is not the way to go!

[quixoten]

I implemented #559 before I saw this PR. It's an alternative implementation for the same issue. I don't know if it resolves the issue in a way that will alleviate @djanowski hesitations, but I offer it up for your consideration in the hopes that it does.