Python: added LMOVE and BLMOVE commands #1536

GilboaAWS · 2024-06-06T12:22:47Z

No description provided.

shohamazon · 2024-06-06T12:53:08Z

python/python/glide/async_commands/core.py

+            >>> result = await client.lmove("testKey1", "testKey2", ListDirection.LEFT, ListDirection.LEFT)
+            >>> assert result == "one"


Suggested change

>>> result = await client.lmove("testKey1", "testKey2", ListDirection.LEFT, ListDirection.LEFT)

>>> assert result == "one"

>>> await client.lmove("testKey1", "testKey2", ListDirection.LEFT, ListDirection.LEFT)

"one"

shohamazon · 2024-06-06T12:54:04Z

python/python/glide/async_commands/core.py

+            >>> updated_array1 = await client.lrange("testKey1", 0, -1)
+            >>> updated_array2 = await client.lrange("testKey2", 0, -1)
+            >>> assert updated_array1 == ["two"]
+            >>> assert updated_array2 == ["one", "three", "four"]


Suggested change

>>> updated_array1 = await client.lrange("testKey1", 0, -1)

>>> updated_array2 = await client.lrange("testKey2", 0, -1)

>>> assert updated_array1 == ["two"]

>>> assert updated_array2 == ["one", "three", "four"]

>>> await client.lrange("testKey1", 0, -1)

["two"]

>>> await client.lrange("testKey2", 0, -1)

["one", "three", "four"]

shohamazon · 2024-06-06T12:56:30Z

python/python/glide/async_commands/core.py

+            >>> result = await client.blmove("testKey1", "testKey2", ListDirection.LEFT, ListDirection.LEFT, 0.1)
+            >>> assert result == "one"
+            >>> updated_array1 = await client.lrange("testKey1", 0, -1)
+            >>> updated_array2 = await client.lrange("testKey2", 0, -1)
+            >>> assert updated_array1 == ["two"]
+            >>> assert updated_array2 == ["one", "three", "four"]


Suggested change

>>> result = await client.blmove("testKey1", "testKey2", ListDirection.LEFT, ListDirection.LEFT, 0.1)

>>> assert result == "one"

>>> updated_array1 = await client.lrange("testKey1", 0, -1)

>>> updated_array2 = await client.lrange("testKey2", 0, -1)

>>> assert updated_array1 == ["two"]

>>> assert updated_array2 == ["one", "three", "four"]

>>> await client.blmove("testKey1", "testKey2", ListDirection.LEFT, ListDirection.LEFT, 0.1)

"one"

>>> await client.lrange("testKey1", 0, -1)

["two"]

>>> await client.lrange("testKey2", 0, -1)

["one", "three", "four"]

shohamazon · 2024-06-06T12:57:21Z

python/python/glide/async_commands/transaction.py

+        of the list stored at `destination` depending on `whereto`.
+        `blmove` is the blocking variant of `lmove`.
+
+        See https://redis.io/commands/blmove/ for details.


Suggested change

See https://redis.io/commands/blmove/ for details.

See https://valkey.io/commands/blmove/ for details.

shohamazon · 2024-06-06T12:57:29Z

python/python/glide/async_commands/transaction.py

+        Notes:
+            When in cluster mode, both `source` and `destination` must map to the same hash slot.
+
+        See https://redis.io/commands/lmove/ for details.


Suggested change

See https://redis.io/commands/lmove/ for details.

See https://valkey.io/commands/lmove/ for details.

shohamazon · 2024-06-06T12:57:44Z

python/python/glide/async_commands/core.py

+            1. When in cluster mode, both `source` and `destination` must map to the same hash slot.
+            2. `blmove` is a client blocking command, see https://github.com/aws/glide-for-redis/wiki/General-Concepts#blocking-commands for more details and best practices.
+
+        See https://redis.io/commands/blmove/ for details.


Suggested change

See https://redis.io/commands/blmove/ for details.

See https://valkey.io/commands/blmove/ for details.

shohamazon · 2024-06-06T12:57:55Z

python/python/glide/async_commands/core.py

+        depending on `wherefrom`, and pushes the element at the first/last element of the list
+        stored at `destination` depending on `whereto`.
+
+        See https://redis.io/commands/lmove/ for details.


Suggested change

See https://redis.io/commands/lmove/ for details.

See https://valkey.io/commands/lmove/ for details.

shohamazon · 2024-06-06T12:58:28Z

python/python/glide/async_commands/core.py

+        wherefrom: ListDirection,
+        whereto: ListDirection,


in python the syntax is:

Suggested change

wherefrom: ListDirection,

whereto: ListDirection,

where_from: ListDirection,

where_to: ListDirection,

shohamazon · 2024-06-06T13:02:34Z

python/python/glide/async_commands/core.py

+            whereto (ListDirection): The direction to add the element to (`ListDirection.LEFT` or `ListDirection.RIGHT`).
+
+        Returns:
+            Optional[str]: The popped element, or `None` if `source` does not exist.


Suggested change

Optional[str]: The popped element, or `None` if `source` does not exist.

Optional[str]: The popped element, or None if `source` does not exist.

shohamazon · 2024-06-06T13:03:37Z

python/python/glide/async_commands/core.py

+        self,
+        source: str,
+        destination: str,
+        wherefrom: ListDirection,


shohamazon · 2024-06-06T13:06:32Z

python/python/glide/async_commands/core.py

+        Blocks the connection until it pops atomically and removes the left/right-most element to the
+        list stored at `source` depending on `wherefrom`, and pushes the element at the first/last element
+        of the list stored at `destination` depending on `whereto`.
+        `blmove` is the blocking variant of `lmove`.


Suggested change

`blmove` is the blocking variant of `lmove`.

`BLMOVE` is the blocking variant of `LMOVE`.

see other blocking commands

acarbonetto · 2024-06-07T01:34:53Z

python/python/glide/async_commands/core.py

+    ) -> Optional[str]:
+        """
+        Atomically pops and removes the left/right-most element to the list stored at `source`
+        depending on `wherefrom`, and pushes the element at the first/last element of the list


if you change as per Sho's suggestions, please update here too

python/python/tests/test_async_client.py

Yury-Fridlyand

Please add changelog entry

python/python/glide/async_commands/core.py

Yury-Fridlyand · 2024-06-07T03:49:40Z

python/python/glide/async_commands/transaction.py

+        Notes:
+            When in cluster mode, both `source` and `destination` must map to the same hash slot.


We don't add this to transction.
It is applicable to the entire transaction, rather than to a particular command(s) in it.

Please Add since section

python/python/glide/async_commands/transaction.py

python/python/tests/test_async_client.py

python/python/tests/test_transaction.py

CHANGELOG.md

python/python/tests/test_async_client.py

Co-authored-by: Shoham Elias <116083498+shohamazon@users.noreply.github.com>

Python: added LMOVE and BLMOVE commands Co-authored-by: Shoham Elias

Python: added LMOVE and BLMOVE commands

e5992cf

GilboaAWS requested a review from a team as a code owner June 6, 2024 12:22

GilboaAWS added the python Python wrapper label Jun 6, 2024

shohamazon reviewed Jun 6, 2024

View reviewed changes

python/python/glide/async_commands/core.py Outdated

self,

source: str,

destination: str,

wherefrom: ListDirection,

Copy link

Collaborator

shohamazon Jun 6, 2024

Choose a reason for hiding this comment

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

same

shohamazon reviewed Jun 6, 2024

View reviewed changes

acarbonetto reviewed Jun 7, 2024

View reviewed changes

acarbonetto approved these changes Jun 7, 2024

View reviewed changes

Yury-Fridlyand reviewed Jun 7, 2024

View reviewed changes

Yury-Fridlyand requested a review from aaron-congo June 7, 2024 03:53

Addressed comments from PR

80aabb3

valkey-io deleted a comment from shohamazon Jun 9, 2024

GilboaAWS and others added 2 commits June 9, 2024 13:21

Merge branch 'aws:main' into lmove_python

c2f99a2

Updated changelog file

d63be4f

shohamazon approved these changes Jun 9, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

python/python/tests/test_async_client.py Outdated Show resolved Hide resolved

python/python/tests/test_async_client.py Outdated Show resolved Hide resolved

Apply suggestions from code review

34d8682

Co-authored-by: Shoham Elias <116083498+shohamazon@users.noreply.github.com>

GilboaAWS merged commit ddc1425 into valkey-io:main Jun 9, 2024
6 checks passed

GilboaAWS deleted the lmove_python branch June 9, 2024 15:08

cyip10 pushed a commit to Bit-Quill/valkey-glide that referenced this pull request Jun 24, 2024

Python: added LMOVE and BLMOVE commands (valkey-io#1536)

601d42b

Python: added LMOVE and BLMOVE commands Co-authored-by: Shoham Elias

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Python: added LMOVE and BLMOVE commands #1536

Python: added LMOVE and BLMOVE commands #1536

GilboaAWS commented Jun 6, 2024

shohamazon Jun 6, 2024

shohamazon Jun 6, 2024

shohamazon Jun 6, 2024

shohamazon Jun 6, 2024

shohamazon Jun 6, 2024

shohamazon Jun 6, 2024

shohamazon Jun 6, 2024

shohamazon Jun 6, 2024

shohamazon Jun 6, 2024

shohamazon Jun 6, 2024

shohamazon Jun 6, 2024

acarbonetto Jun 7, 2024

Yury-Fridlyand left a comment

Yury-Fridlyand Jun 7, 2024

		>>> result = await client.lmove("testKey1", "testKey2", ListDirection.LEFT, ListDirection.LEFT)
		>>> assert result == "one"

	See https://redis.io/commands/blmove/ for details.
	See https://valkey.io/commands/blmove/ for details.

	See https://redis.io/commands/lmove/ for details.
	See https://valkey.io/commands/lmove/ for details.

	Optional[str]: The popped element, or `None` if `source` does not exist.
	Optional[str]: The popped element, or None if `source` does not exist.

	`blmove` is the blocking variant of `lmove`.
	`BLMOVE` is the blocking variant of `LMOVE`.

		Notes:
		When in cluster mode, both `source` and `destination` must map to the same hash slot.

Python: added LMOVE and BLMOVE commands #1536

Python: added LMOVE and BLMOVE commands #1536

Conversation

GilboaAWS commented Jun 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yury-Fridlyand left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment