Python: add BZMPOP command #1412
Conversation
|
|
||
| `BZMPOP` is the blocking variant of `ZMPOP`. | ||
|
|
||
| `BZMPOP` is a client blocking command, see https:/aws/glide-for-redis/wiki/General-Concepts#blocking-commands for more details and best practices. |
There was a problem hiding this comment.
so about #1399 (comment)
I think because we have some commands with this type of documentation (maybe unlink if i remember right)
I think it wont hurt if we add it, it still looks okay in pycharm and it definitely looks better in vscode, so well me what you think
There was a problem hiding this comment.
Either way is fine with me. How about we leave it as is and get Bar's opinion, then if we decide to switch to markdown-style links we can update all the links at once
| """ | ||
| Defines which elements to pop from a sorted set. | ||
|
|
||
| ScoreFilter is a mandatory option for BZMPOP (https://valkey.io/commands/bzmpop). |
There was a problem hiding this comment.
| ScoreFilter is a mandatory option for BZMPOP (https://valkey.io/commands/bzmpop). | |
| ScoreFilter is a mandatory option for [BZMPOP](https://valkey.io/commands/bzmpop). |
There was a problem hiding this comment.
Same as above, if its alright with you I'll leave this as is until we've chatted with Bar and settled on a decision
| modifier: ScoreFilter, | ||
| timeout: float, | ||
| count: Optional[int] = None, | ||
| ) -> Optional[List[Union[str, Dict[str, float]]]]: |
There was a problem hiding this comment.
I wonder if we should create a special object for this in redis-rs, since Java returns a list of object and it doesn't looks nice
There was a problem hiding this comment.
I'm not sure I understand. Are you suggesting we change the data format of the returned object somewhere in redis-rs? Is there an existing example of your suggestion that I can take a look at? One thing we could do to make this a bit clearer is create a Python type alias (see here) and put it in a new file called types.py? Let me know what you think
There was a problem hiding this comment.
I am working on GeoSearch command (and also LCS, it has the same complexity), if you take a look at it, it has a super complicated return type, so I thought about adding a type to Redis Value in redis-rs named geosearch or something, and for this command I will simply return List[GeoSearch], (same udea as we imported script - if you ask how can we do that convertion from rust to any language)
I guess here its not so necessary, since the return type is consistent, but ig we will discuss that later
There was a problem hiding this comment.
@shohamazon
We can create a simple object with (nullable) optional entries (say: GeoLocation) and a constructor that takes an Object[]. This would work as long as one of WITHDIST, WITHCOORD, WITHHASH is included.
4) 1) "edge1"
2) "279.7405"
3) 1) "12.7584877610206604"
2) "38.78813451624225195"
cc: @tjzhang-BQ
| @@ -282,6 +284,16 @@ async def transaction_test( | |||
| args.append("0-2") | |||
| transaction.xtrim(key11, TrimByMinId(threshold="0-2", exact=True)) | |||
| args.append(1) | |||
|
|
|||
| min_version = "7.0.0" | |||
| if not await check_if_server_version_lt(redis_client, min_version): | |||
There was a problem hiding this comment.
use the same key for zmpop and bzmpop
aaron-congo
force-pushed
the
python/integ_acongo_bzmpop
branch
2 times, most recently
from
d43940e
to
2b899b3
Compare
| from datetime import datetime, timedelta | ||
| from enum import Enum | ||
| from typing import ( | ||
| Dict, | ||
| List, | ||
| Mapping, |
There was a problem hiding this comment.
According to the Python docs here, typing.Mapping is a deprecated alias for collections.abc.Mapping, so I changed to use the latter
aaron-congo
force-pushed
the
python/integ_acongo_bzmpop
branch
from
2b899b3
to
1de09d4
Compare
* Python: add BZMPOP command (#273) * Update CHANGELOG with PR link * Apply relevant PR suggestions from other PRs * PR suggestions * Use Mapping instead of Dict * Minor documentation update * PR suggestions * Fix mypy errors * Fix tests * Minor doc change * Align docs and cross-slot tests with recent changes * Minor doc fix * Only run cross-slot test for bzmpop if redis version is >= 7.0.0 * Fix formatting * Fix mypy * Fix crossslot test
Issue #, if available:
N/A
Description of changes:
https://redis.io/docs/latest/commands/bzmpop/
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.