redis-rb / redis-client

Simple low level client for Redis 6+
MIT License
125 stars 60 forks source link

Treat call keyword arguments as Redis flags #18

Closed casperisfine closed 2 years ago

casperisfine commented 2 years ago

Following the discussion and experience gathered in https://github.com/mperham/sidekiq/pull/5298, I think that if we handled keyword arguments as Redis flags, we could significantly ease the transition.

e.g.

redis_rb.set("key", "value", ex: 42, nx: true)

Could be done as:

redis_client.call("SET", "key", "value", ex: 42, nx: true)

Instead of:

redis_client.call("SET", "key", "value", "EX", 42, "nx")

With such feature, a simple method_missing based delegator could achieve a fairly large compatibility with the redis gem.

class CompatClient
  def initialize(client)
    @client = client
  end

  private

  def method_missing(...)
    @client.call(...)
  end
end

redis = CompatClient.new(RedisClient.new)
redis.set("key", "value", ex: 42, nx: true) # => "OK"

Footguns

The one thing that worries me is the Ruby 2 vs Ruby 3 keyword argument semantic.

def call(*args, **kwargs)
  p [args, kwargs]
end

# Ruby 2
call("hash", "foo" => "bar") # => [["hash", {"foo"=>"bar"}], {}]
# Ruby 3
call("hash", "foo" => "bar") # => [["hash"], {"foo"=>"bar"}]
call("hash", { "foo" => "bar" }) # => [["hash", {"foo"=>"bar"}], {}]

But that's limited to string hash literals, so likely only used for HMSET, I think a warning in the readme might be enough.

cc @etiennebarrie

Also cc @mperham as I'd love your thoughts on wether you think it would make redis-client more usable for you.

mperham commented 2 years ago

Aside from the question of a compatibility / migration layer, I'm quite happy with the new gem. The code is simpler, sticks very closely with the documented Redis API and should automatically support new commands as they come and go. I would love to see a performance improvement too but it's new and there's always hiredis.

Ultimately I worry about Sidekiq's dependencies and how well they are being maintained. I think this gem is better suited for long-term support and maintenance and that's why I've been aggressively working to use it.

casperisfine commented 2 years ago

I would love to see a performance improvement too but it's new and there's always hiredis.

Actually, since redis-client's binding support SSL (contrary to hiredis-rb). I'm tempted to enable the hiredis driver by default.

mperham commented 2 years ago

I prefer Ruby infrastructure use pure Ruby by default; it is safer and does not require a compiler/dev env to install. Let the app developer opt into native code if they need that last bit of performance. 90% do not.