search

seomoz / qless-core

Core Lua Scripts for qless
MIT License
85 stars 33 forks source link

Check both 'nil' and 'false' in redis.call return value #64

Closed diesse closed 7 years ago

diesse commented 8 years ago

In redis 3.2, lua script evaluates redis 'nil' value as 'false' instead of 'nil' This causes lua errors as described in #63, since testing for 'nil' fails and empty parameters can be passed to functions

neilmb commented 8 years ago

This looks good to me, but we don't have an automated way of testing on Redis 3.2. Did you run make test locally with the new Redis?

@b4hand Any thoughts on getting another Redis version into the Travis build matrix?

diesse commented 8 years ago

Yes, all 240 tests completed successfully ( on Redis 3.2.1 )

dlecocq commented 8 years ago

We should totally build matrix the Redis version.

pintsized commented 8 years ago

Can this be merged or is it waiting on further testing?

andrewblim commented 7 years ago

I'll echo @pintsized's questions, I'd be in favor of merging this unless others have objections

bkirz commented 7 years ago

Thanks for this fix, and apologies for letting it languish for so long! I've merged #68, which adds redis 3.2.7 to our build matrix. I've rebased this branch on top of that change and have confirmed that this fixes the 3.2.7 tests. Here's the corresponding travis build: https://travis-ci.org/seomoz/qless-core/builds/207219314

Thanks again!