debasishg / scala-redis

A scala library for connecting to a redis server, or a cluster of redis nodes using consistent hashing on the client side.
1.02k stars 219 forks source link

evalSHA with script the returns String #161

Open sdale511 opened 8 years ago

sdale511 commented 8 years ago

Seems to be an issue with evalSHA returning a simple string.

The loaded script should return "foo", but instead returns "[B@7574e223".

The following is run against the head of the master branch:

scala> import com.redis. import com.redis.

scala> val r = new RedisClient("localhost", 6379) r: com.redis.RedisClient = localhost:6379

scala> r.scriptLoad("return 'foo'").get res0: String = 6dcea60f8ec363f0f04e8bc9cbb06f23add0b470

scala> r.evalSHA("6dcea60f8ec363f0f04e8bc9cbb06f23add0b470", List(), List()) res1: Option[String] = Some([B@7574e223)

scala> r.evalBulk("return 'foo'", List(), List()) res2: Option[String] = Some(foo)

debasishg commented 8 years ago

Thanks for reporting .. will look into it ..

debasishg commented 8 years ago

Had a look today into this. Looks like the fix for https://github.com/debasishg/scala-redis/issues/128 did only solve the issue partially. It fixed for integers but broke others. In fact having a generic fix will take some time as the conversion between Redis and Lua data types are quite involved. See http://redis.io/commands/eval for details. I need some time to work on this fix. I am right now tied up with commitments and will be traveling for conferences next week. Will be back in first week of Feb and will fix it after that. Meanwhile if someone can look into it and have a PR, I can take a look.

sdale511 commented 8 years ago

Thanks - we are in the middle of a global release and I have a workaround for now.  If my list clears up before next week week I’ll take a deeper look.

Appreciate the effort (and library). --  Scott Dale

On January 24, 2016 at 8:26:26 AM, Debasish Ghosh (notifications@github.com) wrote:

Had a look today into this. Looks like the fix for #128 did only solve the issue partially. It fixed for integers but broke others. In fact having a generic fix will take some time as the conversion between Redis and Lua data types are quite involved. See http://redis.io/commands/eval for details. I need some time to work on this fix. I am right now tied up with commitments and will be traveling for conferences next week. Will be back in first week of Feb and will fix it after that. Meanwhile if someone can look into it and have a PR, I can take a look.

— Reply to this email directly or view it on GitHub.

debasishg commented 8 years ago

found some time last evening and started working on a branch .. have a look at this commit .. https://github.com/debasishg/scala-redis/commit/c040f223ad46d05ce779b17803348d17e43a743a .. now works with String, Int etc. from Lua. But the API for evalSHA is now less typed. Will continue to work when I get bits of time ..

sdale511 commented 8 years ago

We have put this branch into testing and it looks good.

Thanks.

debasishg commented 8 years ago

cool .. I will test it a bit more and then possibly merge with the master. Thanks for the feedback ..

Uxio0 commented 8 years ago

This should fix it and shouldn't break anyone's code, it's working for us

171