search

redis-rb / redis-client

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

command_builder should should allow SADD to pass an array as an argument #76

Closed jpaas closed 1 year ago

jpaas commented 1 year ago

Using 0.11.2 when using the SADD command, if I pass an array as an argument I get this error: Unsupported command argument type: Array because he argument validation in command_builder does not allow this.

Fix is trivial. PR to follow.

casperisfine commented 1 year ago

Would you mind showing the code that cause this error?

casperisfine commented 1 year ago 
>> RedisClient.new.call("SADD", "set", ["a", "b"])
=> 2

The above works just fine, so I assume you have deeply nested arrays? If so they're rejected on purpose.

jpaas commented 1 year ago

Its coming from sidekiq-batch on this line https://github.com/breamware/sidekiq-batch/blob/master/lib/sidekiq/batch.rb#L99

jpaas commented 1 year ago

Oh maybe you're right. They might be placing their array of job ids in an array. In fact they made this change recently on Sep 28.

https://github.com/breamware/sidekiq-batch/commit/6c4ca7e0cb9a93e7ec3f28330a8580c995d380fe

casperisfine commented 1 year ago

Yup, they should flatten that.

The reason I don't want to allow nested arrays is that most of the time it's not intended. redis-client tries to be strict to avoid silent issues.

jpaas commented 1 year ago

Thanks @casperisfine. I'll take this up with sidekiq-batch