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

Add support for redis-py 3+ #62

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

YuriHeupa
Copy link

redis-py 3.X introduced some breaking changes that made this project incompatible, this PR adds support for these changes and fix tests were applicabble. Follows some redis-py changes:

https://github.com/andymccurdy/redis-py#mset-msetnx-and-zadd

MSET, MSETNX and ZADD
These commands all accept a mapping of key/value pairs. In redis-py 2.X this mapping could be specified as *args or as **kwargs. Both of these styles caused issues when Redis introduced optional flags to ZADD. Relying on *args caused issues with the optional argument order, especially in Python 2.7. Relying on **kwargs caused potential collision issues of user keys with the argument names in the method signature.

https://github.com/andymccurdy/redis-py#encoding-of-user-input

redis-py 3.0 only accepts user data as bytes, strings or numbers (ints, longs and floats). Attempting to specify a key or a value as any other type will raise a DataError exception.

https://github.com/andymccurdy/redis-py#zincrby

redis-py 2.X accidentily modified the argument order of ZINCRBY, swapping the order of value and amount

So this PR change all ZADD calls to supply keys and values as a dict instead of positional arguments, and it also update tests to provide member_data as string instead of dict, as stated in docstring methods, these data should be provided in string already.

Fixes #61

@@ -119,7 +116,7 @@ def rank_member_across(
@param score [float] Member score.
@param member_data [String] Optional member data.
'''
for leaderboard_name in leaderboards:
for leaderboard in leaderboards:
Copy link
Author

Choose a reason for hiding this comment

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

Btw, it also fixes a typo on this function.

Change ZADD calls to supply keys and values as a dict instead of positional arguments.

Update tests to provide member data as string instead of dict, since redis-py 3+ only accepts data as bytes, strings or numbers.

Fixes agoragames#61
@YuriHeupa YuriHeupa force-pushed the add-redis-py-3-support branch from 575e082 to a34e441 Compare January 12, 2021 01:30
@YuriHeupa
Copy link
Author

Rebased and resolved conflicts.

Base automatically changed from master to main March 11, 2021 16:12
@tito
Copy link

tito commented Mar 19, 2021

Could this be packaged in a new version ? Just pip install redis breaks usage of leaderboard

@maxifom
Copy link

maxifom commented Apr 30, 2021

+1

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.

Breaking changes in Redis-Py 3+
4 participants