Initial draft on PUBSUB API and tests #879

ikolomi · 2024-01-30T15:58:21Z

Issue #, if available:
#218

Description of changes:
This is not compilable draft on PUBSUB APIs (which we already had couple of iteration on with the team) and proposed tests that formalize and validate the semantics of the functionality

The main focus should be put on calculate_expected_messages() method that calculates the expected messages propagation for a given configuration

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

ikolomi · 2024-01-30T15:59:44Z

python/python/tests/test_pubsub.py

+                    request=request))
+                all_clients[-1].connect_and_subscribe()
+
+        TestPubSub.publish_and_assert(all_clients)


TODO: Add unsubscription tests

shachlanAmazon · 2024-01-31T11:40:38Z

python/python/glide/async_commands/core.py

+
+    @dataclass
+    class PubSubMsg:
+        """ Describes incomming message """


incomming -> incoming

shachlanAmazon · 2024-01-31T11:43:34Z

python/python/tests/test_pubsub.py

+
+
+MessagesByExactChannel = Dict[str, Dict[str, int]] # channel -> {msg -> cnt}
+MessagesByChannelAndChannel = Dict[Tuple[str, Optional[str]], Dict[str, int]] # (channel, pattern) -> {msg -> cnt}


ByChannelAndPattern?

barshaul · 2024-01-31T16:38:13Z

python/python/glide/async_commands/core.py

@@ -1232,3 +1234,104 @@ async def zrem(
            int,
            await self._execute_command(RequestType.Zrem, [key] + members),
        )
+
+    @dataclass
+    class PubSubMsg:


docstring example for class' args in python:

@dataclass class TestClass: """This is a test class for dataclasses. This is the body of the docstring description. Args: var_int (int): An integer. var_str (str): A string. """

what style it it? I find it more complicated then the proposed form. What is the difference?

this is one of python's most common docstring formats that we use throughout the project
https://www.sphinx-doc.org/en/master/usage/extensions/example_google.html#example-google

there is no example for the dataclass in the sphinx doc, "args" are also not applicable to dataclass. I`ll follow the chat-gtp proposed style

barshaul · 2024-01-31T16:39:46Z

python/python/glide/async_commands/core.py

+        Pattern = 2
+        """ Use channel name patterns """
+        Sharded = 3
+        """ Use sharded PUBSUB. See  https://redis.io/docs/interact/pubsub/#sharded-pubsub for more details """


minor: remove redundant space

barshaul · 2024-01-31T16:56:36Z

python/python/glide/async_commands/core.py

+                        channels_or_patterns: Set[str],
+                        channel_mode: ChannelModes,
+                        callback: Callable[[PubSubMsg, Optional[Any]]],
+                        context: Optional[Any]) -> None:


what should the context be?

anything to pass to callback together with the new message

so it isn't clear from this documentation:

context: User-provided context for this subscription

also the typing seems off, it should be Callable[[arg_type1, arg_type2], return_type]

barshaul · 2024-01-31T17:41:12Z

python/python/glide/async_commands/core.py

+            context: User-provided context for this subscription        
+
+        Examples:
+            >>> await client.subscribe({"local-news"}, ChannelModes.Exact, process_news_message, None)


I think we can add a simple example for process_news_message too

barshaul · 2024-01-31T17:42:20Z

python/python/glide/async_commands/core.py

+        See https://redis.io/docs/interact/pubsub for more details.
+
+        Args:
+            channels_or_patterns: Set of channels or patterns to subscibe to.


the args docstring should include typing

barshaul · 2024-01-31T17:43:19Z

python/python/glide/async_commands/core.py

+    async def publish(self,
+                      message: str,
+                      channels: Set[str],
+                      sharded: bool = False) -> int:


why is the API different here? with sharded as a bool and all

shouldn't we review this file?

I am not following - we are reviewing this file :) Different from what

oh, my bad. publish can't be done with patterns. this is why the api is different from subscribe :)

barshaul · 2024-01-31T17:54:50Z

python/python/tests/test_pubsub.py

+            self.cluster_mode = cluster_mode
+            self.request = request
+
+            self.redis_client: CoreCommands = None


CoreCommands? should be TRedisClient

why do I need union? - I am only using the CoreCommands methods?

barshaul · 2024-01-31T18:00:16Z

python/python/tests/test_pubsub.py

+        @staticmethod
+        def handle_new_message(message: CoreCommands.PubSubMsg, client: TestPubSub.PubSubClient) -> None:
+            key = (message.channel, message.pattern)
+            if key not in client.messages_received:


key = (message.channel, message.pattern) channel_or_pattern_msgs = client.messages_received.setdefault(key, {}) if message not in channel_or_pattern_msgs: ...

barshaul · 2024-01-31T18:02:56Z

python/python/glide/async_commands/core.py

+                        channels_or_patterns: Set[str],
+                        channel_mode: ChannelModes,
+                        callback: Callable[[PubSubMsg, Optional[Any]]],
+                        context: Optional[Any]) -> None:


why returning None and not OK?

dont we use Exceptions to signal errors?

just saw redis' doc "When successful, this command doesn't return anything", so ignore this comment

barshaul · 2024-01-31T18:03:14Z

python/python/tests/test_pubsub.py

+            client.messages_received[key][message] += 1
+
+
+        async def connect_and_subscribe(self) -> List[Any]:


fix return type

barshaul · 2024-01-31T18:05:40Z

python/python/tests/test_pubsub.py

+        async def connect_and_subscribe(self) -> List[Any]:
+            self.redis_client = await create_client(self.request, self.cluster_mode)
+
+            for channel_mode, channels_or_patterns in self.subsciptions:


in self.subsciptions.items()

shachlanAmazon

missing tests for re-subscription -

subscribe to channel foo with callback a & context a', subscribe to channel foo with callback b & context b', verify that only b is called with b'.
subscribe to channels foo & bar, subscribe to channel foo, check that callback a is called for messages on bar, callback b for messages on foo.
what should happen if you subscribe to pattern foo, and then subscribe to channel foo?

probably more combination of actions that I didn't consider now.

python/python/glide/async_commands/core.py

shachlanAmazon · 2024-02-01T09:14:20Z

python/python/glide/async_commands/core.py

+        meaning subsequent subsciption for the same channel/pattern will override the previous association, dereferencing the original callback object
+        Note1: Overlapping subscriptions produced by combinations of Exact and Pattern modes
+        count as distinc subscriptions and will produce duplicated messages.
+        Note2: Patterns are not applicable in Sharded mode. 


This isn't very clear, for anyone not familiar with the details. If you want to say this, it needs to be explained better.
Maybe
"Patterns are not applicable in Sharded mode. That is, it is not possible to create a sharded subscription with a pattern, and pattern/exact subscriptions won't receive messages which were published with `spublish`"

or some similar phrasing.

shachlanAmazon · 2024-02-01T09:28:26Z

python/python/glide/async_commands/core.py

+
+        Args:
+            message: Message to publish
+            channels: Set of channels or patterns to publish the message on.


AFAIS Both PUBLISH & SPUBLISH only publish on a single channel, not a set. neither publishes on patterns. Do you want to expand our functionality beyond what Redis offers? why?

https://redis.io/commands/publish/
https://redis.io/commands/spublish/

No, I dont want, the arg should be a single channel

shachlanAmazon · 2024-02-01T09:28:48Z

python/python/glide/async_commands/core.py

+            sharded: Use sharded PUBSUB mode.
+
+        Returns:
+            int: Number of clients that received the message. //TODO: Consider None, do we really want it?


It's the Redis API, why give less?

Its a weak hint, but I agree

shachlanAmazon · 2024-02-01T09:29:42Z

python/python/glide/async_commands/core.py

+        """
+        pass
+
+    # async def get_pubsub_message(self) -> PubSubMsg:


shachlanAmazon · 2024-02-01T10:19:36Z

python/python/tests/test_pubsub.py

+
+        def calculate_expected_messages(self, all_clients: List[TestPubSub.PubSubClient]) -> None:
+            for sender in all_clients:
+                for sharded_channel, sharded_channel_messages in sender.sharded_messages_to_publish.items():


I believe the nested loops can be flattened using flat_map or list comprehensions.

it becomes harder to understand the code

shachlanAmazon · 2024-02-01T10:20:30Z

python/python/tests/test_pubsub.py

+        """ PUBSUB basic happy case using exact channel names - tests that clients receive all the messages using exact subscribe to a single channel """
+
+        CHANNEL_NAME = "test-channel"
+        CLIENTS_COUNT = 5


the simple tests should have only a single client - if they fail, we want debugging to also be simple.

The easiest functional setting is 2, will change to that

why is 2 easier than 1?

of course it is not easier, it is more functional

what does "more functional" mean here?
it does more? I agree, but a test should do as little as possible, in order to focus the coder on the source of the error if the test fails. In that sense, adding also gets, sets, and other calls to this test would make it more functional, but it doesn't help the test be focused.

I do not agree that the test has to do as little as possible. The value of such tests are pretty low.

I also think we shall have a basic and simple test for each option (sharded/pattern/exact), a simple subscribe publish unsbuscribe. We shall have more complicated tests as well, but having a straightforward simple test will give us better understanding with failures

The value of simple tests is that if they fail, it's easy to understand why they fail. The value of complex tests is that they can catch multiple points of failure. The value of a combination of complex & simple tests is that if the simple test failed, you can start by fixing it, and that will save you time debugging a more complex test - but if the simple test didn't fail, it still saves you time debugging the complex test, because it removes at least one scenario that needs to be considered. If you have multiple simple tests, this value aggregates.

So, we're not saying that you should have only simple tests, but please - ALSO add simple tests.

added based test test_pubsub_basic()

shachlanAmazon · 2024-02-01T10:36:51Z

python/python/tests/test_pubsub.py

+    @pytest.mark.parametrize("cluster_mode", [True, False])
+    @pytest.mark.parametrize("sharded_pubsub", [True, False])
+    async def test_pubsub_channel_boundary_exact(self, request, cluster_mode, sharded_pubsub):
+        """ Tests that messages do not cross channel boundaries by exact subsctiption """


I don't understand what this test does

clients listening on "channel-x" wont get messages published on "channel-y"

why do we need this test? this is enforced by the server.

because we might have bugs...

what bugs do you expect this test to catch? "because we might have bugs" doesn't explain which tests are right and which aren't. The number of tests that haven't been written far exceeds those that have been - there's some kind of criteria that explain which tests are meaningful and which aren't.

Bugs that may lead to leakage of messages across the channels. This is something we have to test.

shachlanAmazon · 2024-02-01T10:37:08Z

python/python/tests/test_pubsub.py

+
+    @pytest.mark.parametrize("cluster_mode", [True, False])
+    async def test_pubsub_channel_boundary_pattern(self, request, cluster_mode):
+        """ Tests that messages do not cross channel boundaries by pattern subsctiption """


same as above but involving patterns

shachlanAmazon · 2024-02-01T10:38:16Z

python/python/tests/test_pubsub.py

+    @pytest.mark.parametrize("cluster_mode", [True, False])
+    @pytest.mark.parametrize("sharded_pubsub", [True, False])
+    async def test_pubsub_callbacks_per_channel(self, request, cluster_mode, sharded_pubsub):
+        """ Tests that callbacks are per channel """


you should also check that the correct context is passed

shachlanAmazon · 2024-02-02T11:15:26Z

we also need tests for resubscription after disconnects / partial disconnects / failover.

barshaul · 2024-02-04T14:14:25Z

python/python/glide/async_commands/core.py

+        Args:
+            message: Message to publish
+            channels: Set of channels or patterns to publish the message on.
+            sharded: Use sharded PUBSUB mode.


maybe add a link to https://redis.io/commands/spublish/, for users who don't know what sharded pubsub is and what this option really says

elaborated, done

barshaul · 2024-02-04T14:23:07Z

python/python/tests/test_pubsub.py

+                                                  callback=TestPubSub.PubSubClient.handle_new_message,
+                                                  context=self)
+
+        def assert_client(self) -> None:


assert_client => verify_received_expected_messages

rejecting, assert_client is more suitable in my opinion

assert_client - what? the name is doesn't explain what the function does.

it asserts client. Explanatory enough, done

barshaul · 2024-02-04T14:33:24Z

python/python/tests/test_pubsub.py

+                        channel_and_pattern = sharded_channel, None
+                        if channel_and_pattern not in self.messages_to_receive:
+                            self.messages_to_receive[channel_and_pattern] = {}
+                        for message, cnt in sharded_channel_messages.items():


... channel_and_pattern = sharded_channel, None for message, cnt in sharded_channel_messages.items(): channel_messages = self.messages_to_receive.setdefault(channel_and_pattern, {}) channel_messages.setdefault(message, 0) channel_messages[message] += cnt

barshaul · 2024-02-04T14:41:05Z

python/python/tests/test_pubsub.py

+        coroutines = []
+        for client in all_clients:
+            coroutines += client.sched_publish_messages()
+        asyncio.wait(coroutines)


I think https://docs.python.org/3/library/asyncio-task.html#asyncio.gather is what you need here

removed for now, done

barshaul · 2024-02-04T14:43:33Z

python/python/tests/test_pubsub.py

+    @staticmethod
+    async def publish_and_assert(all_clients: List[TestPubSub.PubSubClient]):
+        for client in all_clients:
+            client.publish_messages()


missing await

barshaul · 2024-02-04T15:03:11Z

python/python/tests/test_pubsub.py

+
+        # TODO: Allow async fanning out?
+        def sched_publish_messages(self) -> List[Awaitable[None]]:
+            # coroutines = []


why commented out?

removed, done

barshaul · 2024-02-04T15:07:37Z

python/python/tests/test_pubsub.py

+                                     context=None)
+
+        await redis_client.publish(message="hi", channels={"test-channel-1"}, sharded=sharded_pubsub)
+        # FIXME - wait for receive with TO


take a look on the init_future and init_callback in the create function in redis_client.py, you can use this pattern

i will use a simple async sleep to allow the backend to complete the sending

…assing. 3. Python DEVELOPER.md fixes to ignore .env folder when running linters

shachlanAmazon · 2024-02-12T07:29:00Z

python/python/glide/async_commands/core.py

+            message (str): Incoming message.
+            channel (str): Name of an channel that triggered the message.
+            pattern (Optional[str]): Pattern that triggered the message.
+            context (Optional[str]): User-provided context for this subscription.


why is the context part of the message? IMO it's a break of encapsulation, and not a part of the message concept. The context should be passed as a second argument to the callback, not hidden inside the message.
Also, why is the context a string?

I think it makes the usage more convenient - no need to access 2 params in the callback. Redis message concept does not mean it should be bit-exact to the API.
Optional[str] is a bug, will change to Optional[Any]
Done

shachlanAmazon · 2024-02-12T07:30:07Z

python/python/glide/async_commands/core.py

+        context: Optional[Any],
+    ) -> None:
+        """
+        Add specific channels or set of channels defined by a pattern to active subscriptions.


specific channels or set of channels - what does this mean? what's the difference between specific channels and a set of channels?

read the whole sentence - "...or set of channels defined by a pattern"

rephrased - "Add specific channels or channels patterns to active subscriptions."
Done

shachlanAmazon · 2024-02-12T07:30:48Z

python/python/glide/async_commands/core.py

+        meaning subsequent subsciption for the same channel/pattern will override the previous association,
+        dereferencing the original callback object.
+        Note1: Overlapping subscriptions produced by combinations of Exact and Pattern modes
+        count as distinc subscriptions and will produce duplicated messages.


distinc -> distinct

shachlanAmazon · 2024-02-12T07:40:43Z

python/python/glide/async_commands/core.py

+        Note1: Overlapping subscriptions produced by combinations of Exact and Pattern modes
+        count as distinc subscriptions and will produce duplicated messages.
+        Note2: Patterns are not applicable in Sharded mode. That is, it is not possible to create a sharded subscription with a pattern,
+        and pattern/exact subscriptions won't receive messages which were published with using Sharded mode.


published with using Sharded mode -> published in Sharded mode

shachlanAmazon · 2024-02-12T13:20:19Z

missing test: client using RESP2 receives informative error when trying to subscribe, no subscription is created, and connection remains valid.

barshaul · 2024-02-13T13:40:24Z

python/python/glide/async_commands/core.py

+
+    @dataclass
+    class PubSubMsg:
+        """Describes incoming message


minor: Describes incoming message => Describes incoming PubSub message

barshaul · 2024-02-13T13:41:37Z

python/python/glide/async_commands/core.py

+
+        Attributes:
+            message (str): Incoming message.
+            channel (str): Name of an channel that triggered the message.


Name of an channel that triggered the message. => The name of the channel that triggered the message.

barshaul · 2024-02-13T13:41:56Z

python/python/glide/async_commands/core.py

+        """Describes incoming message
+
+        Attributes:
+            message (str): Incoming message.


Incoming message. => Incoming message content

barshaul · 2024-02-13T13:43:32Z

python/python/glide/async_commands/core.py

+        Attributes:
+            message (str): Incoming message.
+            channel (str): Name of an channel that triggered the message.
+            pattern (Optional[str]): Pattern that triggered the message.


The channel pattern that triggered the message.

How can we have both the channel name and a pattern? shouldn't it be only one of pattern/channel?

When subscribed to a pattern, the channel name is being received with new messages?

barshaul · 2024-02-13T13:47:32Z

python/python/glide/async_commands/core.py

@@ -1368,3 +1373,111 @@ async def invoke_script(
                ["foo", "bar"]
        """
        return await self._execute_script(script.get_hash(), keys, args)
+
+    @dataclass
+    class PubSubMsg:


General note: lets move all pubsub code to a dedicated pubusb.py file under the async_commands folder

barshaul · 2024-02-13T13:50:10Z

python/python/glide/async_commands/core.py

+            channels_or_patterns (Set[str]): Set of channels or patterns to subscibe to.
+            channel_mode (ChannelModes): Mode of operation. See ChannelModes.
+            callback (Callable[[CoreCommands.PubSubMsg], None]): Callback to be called for each new message.
+            For each channel or pattern there is exactly one active callback.


barshaul · 2024-02-13T13:51:08Z

python/python/glide/async_commands/core.py

+            context (Optional[Any]): User-provided context for this subscription. Will be passed to callback as part of PubSubMsg.
+
+        Examples:
+            >>> def process_news_message(message: PubSubMsg) -> None:


process_news_message => process_new_message

barshaul · 2024-02-13T13:54:25Z

python/python/glide/async_commands/core.py

+
+        Examples:
+            >>> def process_news_message(message: PubSubMsg) -> None:
+            >>>     print(f"'{message.context}' received new message '{message.message}' from channel '{message.channel}')


It isn't clear from the documentation that the context will be provided as a member of PubSubMsg, and that the member's name is context and so on.. without the example I wouldn't know this is how it should be used.
Why not providing the context as an argument of the callback function?

def process_news_message(message: PubSubMsg, context: Optional[Any]) -> None: print(f"'{context}' received new message '{message.message}' from channel '{message.channel}')

I would do this a more explanatory example:

def process_new_message(message: PubSubMsg, context: Dict[str, List[str]]): context.set_default(message.channel, []) context[message.channel].append(message.message)

barshaul · 2024-02-13T14:07:44Z

python/python/glide/async_commands/core.py

+        Args:
+            channels_or_patterns (Set[str]): Set of channels or patterns to unsubscibe from.
+            Empty set unsubscribes from all channels.
+


nit: remove newline

barshaul · 2024-02-13T14:09:01Z

python/python/glide/async_commands/core.py

+        """
+        ...
+
+    async def unsubscribe(


We are adding the redis documentation for all commands, I see you added only a link to https://redis.io/docs/interact/pubsub. We should have a like to https://redis.io/commands/unsubscribe/ (or other commands) as well

barshaul · 2024-02-13T14:13:18Z

python/python/tests/test_pubsub.py

+        await redis_client.subscribe(
+            channels_or_patterns={CHANNEL_A_NAME},
+            channel_mode=(
+                CoreCommands.ChannelModes.Sharded


minor: Can you import ChannelModes, so the code would be cleaner?

barshaul · 2024-02-13T14:15:54Z

python/python/tests/test_pubsub.py

+        await redis_client.publish(
+            message="hi", channel=CHANNEL_A_NAME, sharded=sharded_pubsub
+        )
+        await asyncio.sleep(1)


from past experience: these pubsub tests with sleep tend to be flakey. Instead, can you wait on a future and pass it as a context?

ikolomi assigned barshaul and asafpamzn Jan 30, 2024

ikolomi requested a review from a team as a code owner January 30, 2024 15:58

ikolomi assigned shachlanAmazon Jan 30, 2024

ikolomi commented Jan 30, 2024

View reviewed changes

Yury-Fridlyand added the python label Jan 30, 2024

shachlanAmazon reviewed Jan 31, 2024

View reviewed changes

barshaul reviewed Jan 31, 2024

View reviewed changes

shachlanAmazon reviewed Feb 1, 2024

View reviewed changes

barshaul reviewed Feb 4, 2024

View reviewed changes

ikolomi force-pushed the pubsub_2 branch from 4153354 to f440160 Compare February 11, 2024 21:46

Initial draft on PUBSUB API and tests

eb05ae9

Added placeholder for test_pubsub_unsubscribe

d879a0e

ikolomi force-pushed the pubsub_2 branch from b5f0051 to b0d50a8 Compare February 11, 2024 22:10

1. Fixes for PR comments. 2. PUBSUB Mock implementation - tests are p…

2006b64

…assing. 3. Python DEVELOPER.md fixes to ignore .env folder when running linters

ikolomi force-pushed the pubsub_2 branch from b0d50a8 to 2006b64 Compare February 11, 2024 22:19

shachlanAmazon reviewed Feb 12, 2024

View reviewed changes

Fixes for PR comments

ebc941d

barshaul reviewed Feb 13, 2024

View reviewed changes



		MessagesByExactChannel = Dict[str, Dict[str, int]] # channel -> {msg -> cnt}
		MessagesByChannelAndChannel = Dict[Tuple[str, Optional[str]], Dict[str, int]] # (channel, pattern) -> {msg -> cnt}

		client.messages_received[key][message] += 1


		async def connect_and_subscribe(self) -> List[Any]:

Initial draft on PUBSUB API and tests #879

Are you sure you want to change the base?

Initial draft on PUBSUB API and tests #879

Conversation

ikolomi commented Jan 30, 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

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

barshaul Jan 31, 2024 • edited

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

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

shachlanAmazon left a comment • edited

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

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

barshaul Feb 4, 2024 • edited

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

ikolomi Feb 1, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shachlanAmazon Feb 4, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

barshaul Jan 31, 2024 •

edited

shachlanAmazon left a comment •

edited

barshaul Feb 4, 2024 •

edited

ikolomi Feb 1, 2024 •

edited

shachlanAmazon Feb 4, 2024 •

edited