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.

Sign up for GitHub

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

#1434 Added support for ZMSCORE new in Redis 6.2 RC #1437

Merged
merged 9 commits into from
Aug 29, 2021
+31 −0
Merged

#1434 Added support for ZMSCORE new in Redis 6.2 RC #1437

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
49af347
#1434 Added zmscore command support
Dec 21, 2020
77eadf3
#1434 Fixed typo and doc
Dec 21, 2020
975a134
Merge branch 'master' into 1434-zmscore
Aug 5, 2021
74f1a7c
Merge remote-tracking branch 'upstream/master' into 1434-zmscore
Aug 7, 2021
234e7d0
#1434 Set [] as default value for members arg in zmscore func
Aug 16, 2021
1cbf7cd
#1434 Set None as default value for members arg in zmscore func
Aug 16, 2021
1448f2c
#1434 Removed default value for members arg in zmscore func
Aug 16, 2021
4981719
#1434 Solve conflict with master
Aug 23, 2021
9bc7507
Fixed flake8 formatting
Aug 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions redis/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,11 @@ def parse_zscan(response, **options):
return int(cursor), list(zip(it, map(score_cast_func, it)))


def parse_zmscore(response, **options):
# zmscore: list of scores (double precision floating point number) or nil
return [float(score) if score is not None else None for score in response]


def parse_slowlog_get(response, **options):
space = ' ' if options.get('decode_responses', False) else b' '
return [{
Expand Down Expand Up @@ -701,6 +706,7 @@ class Redis:
'XPENDING': parse_xpending,
'ZADD': parse_zadd,
'ZSCAN': parse_zscan,
'ZMSCORE': parse_zmscore,
}

@classmethod
Expand Down Expand Up @@ -3360,6 +3366,22 @@ def zunionstore(self, dest, keys, aggregate=None):
"""
return self._zaggregate('ZUNIONSTORE', dest, keys, aggregate)

def zmscore(self, key, members):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def zmscore(self, key, members):
def zmscore(self, key, members=[]):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the late reply.

I am ok with a default value proposal (for compatible reason). But not sure if using a mutable type is fine. This contains risk that the default value might be modified.

So maybe I should change it with a default value None and modify the function body accordingly?

"""
Returns the scores associated with the specified members
in the sorted set stored at key.

``members`` should be a list of the member name.
Return type is a list of score.

If the member does not exist, a None will be returned
in corresponding position.
"""
if not isinstance(members, list) or len(members) < 1:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if not isinstance(members, list) or len(members) < 1:
if not members:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So we should check if it's None or not and if it's a list type data?

Copy link
Contributor

Choose a reason for hiding this comment

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

According to the zmscore docs ZMSCORE requires a minimum of one member. This either means that we either:

  1. Validate that there's a non-zero length list (as above).
  2. Validate that the item passed in is a string or a dictionary.

I'd rather we not overload the members argument, and instead always pass in a list. For the simple case, the list has one item (i.e ['foo']). Validating that we were passed a list I think is overkill, as the docs and the function signature are indicative of the requirement.

raise DataError('ZMSCORE members must be a non-empty list')
pieces = [key] + members
return self.execute_command('ZMSCORE', *pieces)

def _zaggregate(self, command, dest, keys, aggregate=None,
**options):
pieces = [command]
Expand Down
11 changes: 11 additions & 0 deletions tests/test_commands.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1817,6 +1817,17 @@ def test_zunionstore_with_weight(self, r):
assert r.zrange('d', 0, -1, withscores=True) == \
[(b'a2', 5), (b'a4', 12), (b'a3', 20), (b'a1', 23)]

@skip_if_server_version_lt('6.1.240')
def test_zmscore(self, r):
with pytest.raises(exceptions.DataError):
r.zmscore('invalid_key', [])

assert r.zmscore('invalid_key', ['invalid_member']) == [None]

r.zadd('a', {'a1': 1, 'a2': 2, 'a3': 3.5})
assert r.zmscore('a', ['a1', 'a2', 'a3', 'a4']) == \
[1.0, 2.0, 3.5, None]

# HYPERLOGLOG TESTS
@skip_if_server_version_lt('2.8.9')
def test_pfadd(self, r):
Expand Down