Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redis operation that returns dict now converted to Lua table when called inside eval operation #209

Merged

Conversation

NimrodParasol
Copy link
Contributor

During eval operation, results of Redis operations returning dictionaries are now passed as Lua tables instead of python lists
This allows for iteration over them within the Lua script.

This is a fix to the error described in issue 204

…ries are now passed as lua tables instead of python lists
Copy link
Collaborator

@bmerry bmerry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. For consistency with the other tests, can you pass the key being accessed to redis.eval?

@coveralls
Copy link

coveralls commented Aug 20, 2018

Coverage Status

Coverage increased (+0.001%) to 98.484% when pulling b461534 on NimrodParasol:eval-redis-dict-results-lua-objects into c3c45af on jamesls:master.

@NimrodParasol
Copy link
Contributor Author

@bmerry I added the key as you asked. but for future reference please notice that as far as I've seen the tests never use the keys that are passed to them but using hard-coded values within the Lua scripts.

end
return result
"""
val = self.redis.eval(lua, 0, 'foo')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be 1, not 0 (number of keys passed).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are totally right.
Fixed it.

@bmerry
Copy link
Collaborator

bmerry commented Aug 21, 2018

for future reference please notice that as far as I've seen the tests never use the keys that are passed to them but using hard-coded values within the Lua scripts.

I agree, that's not ideal, but it's legal. According to the redis docs, "All Redis commands must be analyzed before execution to determine which keys the command will operate on. In order for this to be true for EVAL, keys must be passed explicitly." It doesn't matter to redis whether the Lua code uses KEYS or hard-codes them, as long as the redis server is informed about all keys that might be accessed.

Copy link
Collaborator

@bmerry bmerry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. I'm going to merge it now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants