XRPLF / xrpl-py

A Python library to interact with the XRP Ledger (XRPL) blockchain
ISC License
144 stars 83 forks source link

[FIX] Issue #709: do not enqueue all responses #713

Open ckeshava opened 4 weeks ago

ckeshava commented 4 weeks ago

High Level Overview of Change

Do not enqueue messages that correspond to an existing request

Context of Change

Bug Fix: At the moment, we are enqueuing all the incoming messages.

Type of Change

Did you update CHANGELOG.md?

ckeshava commented 4 weeks ago

I'm not able to find requests which increase the queue size

class TestWebSocketClient(TestCase):
    """Performace test of the async web_socket_client."""

    def test_web_socket_client(self: TestWebSocketClient) -> None:
        URL = "wss://s.devnet.rippletest.net:51233"

        with WebsocketClient(URL) as client:
            pendings_msg_size = []

            for _ in range(100):
                client.request(ServerInfo())
                pendings_msg_size.append(len(client._open_requests))

            print(max(pendings_msg_size)) # returns 0
mvadari commented 3 weeks ago

I'm not able to find requests which increase the queue size

class TestWebSocketClient(TestCase):
    """Performace test of the async web_socket_client."""

    def test_web_socket_client(self: TestWebSocketClient) -> None:
        URL = "wss://s.devnet.rippletest.net:51233"

        with WebsocketClient(URL) as client:
            pendings_msg_size = []

            for _ in range(100):
                client.request(ServerInfo())
                pendings_msg_size.append(len(client._open_requests))

            print(max(pendings_msg_size)) # returns 0

Try client._messages instead of client._open_requests.

ckeshava commented 3 weeks ago

_messages

client._messages has a qsize of 0

mvadari commented 3 weeks ago

How did you test this change? If not by adding a test case, then what?

ckeshava commented 3 weeks ago

I don't have a good idea to test this change. But irrespective of the impact on this issue, I think this change is useful.

I don't know which inputs would reproduce the issue description.