Skip to content
This repository has been archived by the owner on Sep 11, 2019. It is now read-only.

Improve emulation of redis -> Lua returns #13

Merged
merged 3 commits into from
Feb 20, 2018
Merged

Conversation

bmerry
Copy link

@bmerry bmerry commented Feb 19, 2018

There were some cases omitted from _convert_redis_result e.g.
recursively converting lists. A more serious problem is that
fakenewsredis follows the types returned by redis-py, which in many
cases have a command-specific conversion applied e.g. EXISTS returns
bool instead of 0/1. Since in real redis the Lua script is not subject
to these conversions, we need to "undo" them.

In some cases this can be done generically e.g. bool -> int or float ->
str, but in others we need command-specific conversions. This is done
with the @_lua_reply decorator.

There is probably more work needed to handle all the commands: redis-py
has a large table of conversions, and I've only handled some of the
common cases.

Also fixed flushall to return True (independently of Lua).

There were some cases omitted from _convert_redis_result e.g.
recursively converting lists. A more serious problem is that
fakenewsredis follows the types returned by redis-py, which in many
cases have a command-specific conversion applied e.g. EXISTS returns
bool instead of 0/1. Since in real redis the Lua script is not subject
to these conversions, we need to "undo" them.

In some cases this can be done generically e.g. bool -> int or float ->
str, but in others we need command-specific conversions. This is done
with the `@_lua_reply` decorator.

There is probably more work needed to handle all the commands: redis-py
has a large table of conversions, and I've only handled some of the
common cases.

Also fixed flushall to return True (independently of Lua).
@bmerry
Copy link
Author

bmerry commented Feb 19, 2018

@blueyed @blfoster Github isn't letting me request a PR (I think because it is an organisation repo and you're not in the organisation), but if you have any feedback on this let me know.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 98.051% when pulling c44ae5e on lua-return-types into 1246627 on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 98.051% when pulling c44ae5e on lua-return-types into 1246627 on master.

@coveralls
Copy link

coveralls commented Feb 19, 2018

Coverage Status

Coverage increased (+0.03%) to 98.056% when pulling 2788f83 on lua-return-types into 1246627 on master.

This is to match redis's documented behaviour of sorting some arrays
when Lua fetches them, to make scripts deterministic.
@bmerry bmerry merged commit 35f8948 into master Feb 20, 2018
@bmerry bmerry deleted the lua-return-types branch February 20, 2018 13:27
@blueyed
Copy link

blueyed commented Feb 23, 2018

@bmerry
This works really well for me, thanks for this!

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

Successfully merging this pull request may close these issues.

None yet

3 participants