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.

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

Resolve "Spot websocket connection doesn't get closed properly" #318

Merged
merged 19 commits into from
Nov 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions .github/workflows/_test_futures_private.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,12 @@ jobs:
FUTURES_SECRET_KEY: ${{ secrets.FUTURES_SECRET_KEY }}
FUTURES_SANDBOX_KEY: ${{ secrets.FUTURES_SANDBOX_KEY }}
FUTURES_SANDBOX_SECRET: ${{ secrets.FUTURES_SANDBOX_SECRET }}
run: pytest -vv -m "futures_auth and not futures_websocket and not flaky" tests
run: pytest -vv -m "futures and futures_auth and not futures_websocket and not flaky" tests

- name: Testing Futures websocket client
env:
FUTURES_API_KEY: ${{ secrets.FUTURES_API_KEY }}
FUTURES_SECRET_KEY: ${{ secrets.FUTURES_SECRET_KEY }}
FUTURES_SANDBOX_KEY: ${{ secrets.FUTURES_SANDBOX_KEY }}
FUTURES_SANDBOX_SECRET: ${{ secrets.FUTURES_SANDBOX_SECRET }}
run: pytest -vv -m "futures_websocket and not flaky" tests
run: pytest -vv -m "futures and futures_auth and futures_websocket and not flaky" tests
3 changes: 3 additions & 0 deletions .github/workflows/_test_futures_public.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,6 @@ jobs:

- name: Testing Futures REST endpoints
run: pytest -vv -m "futures and not futures_auth and not futures_websocket" tests

- name: Testing Futures websocket endpoints
run: pytest -vv -m "futures and not futures_auth and futures_websocket" tests
2 changes: 1 addition & 1 deletion .github/workflows/_test_nft_private.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,4 @@ jobs:
env:
SPOT_API_KEY: ${{ secrets.SPOT_API_KEY }}
SPOT_SECRET_KEY: ${{ secrets.SPOT_SECRET_KEY }}
run: pytest -vv -m nft_auth tests
run: pytest -vv -m "nft and nft_auth" tests
8 changes: 7 additions & 1 deletion .github/workflows/_test_spot_private.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,14 @@ jobs:
echo "$env:GITHUB_WORKSPACE\.venv\Scripts" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append
uv pip install ".[test]"

- name: Testing Spot REST endpoints
env:
SPOT_API_KEY: ${{ secrets.SPOT_API_KEY }}
SPOT_SECRET_KEY: ${{ secrets.SPOT_SECRET_KEY }}
run: pytest -vv -m "spot and spot_auth and not spot_websocket" tests

- name: Testing Spot websocket client
env:
SPOT_API_KEY: ${{ secrets.SPOT_API_KEY }}
SPOT_SECRET_KEY: ${{ secrets.SPOT_SECRET_KEY }}
run: pytest -vv -m spot_websocket tests
run: pytest -vv -m "spot and spot_auth and spot_websocket" tests
4 changes: 4 additions & 0 deletions .github/workflows/_test_spot_public.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ jobs:
github.com:443
objects.githubusercontent.com:443
pypi.org:443
ws.kraken.com:443

- name: Checkout repository
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
Expand Down Expand Up @@ -70,3 +71,6 @@ jobs:

- name: Testing Spot REST endpoints
run: pytest -vv -m "spot and not spot_auth and not spot_websocket" tests

- name: Testing Spot websocket endpoints
run: pytest -vv -m "spot and not spot_auth and spot_websocket" tests
6 changes: 4 additions & 2 deletions kraken/futures/websocket/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
from random import random
from typing import TYPE_CHECKING, Any

import websockets
from websockets.asyncio.client import connect

from kraken.exceptions import MaxReconnectError

Expand Down Expand Up @@ -95,9 +95,11 @@ async def __run( # noqa: C901
self.__new_challenge = None
self.__last_challenge = None

async with websockets.connect( # pylint: disable=no-member # noqa: PLR1702
async with connect( # pylint: disable=no-member # noqa: PLR1702
f"wss://{self.__ws_endpoint}",
additional_headers={"User-Agent": "btschwertfeger/python-kraken-sdk"},
ping_interval=30,
max_queue=None, # FIXME: This is not recommended by the docs https://websockets.readthedocs.io/en/stable/reference/asyncio/client.html#module-websockets.asyncio.client
) as socket:
LOG.info("Websocket connected!")
self.socket = socket
Expand Down
5 changes: 3 additions & 2 deletions kraken/spot/websocket/connectors.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from time import time
from typing import TYPE_CHECKING, Any, Final

import websockets
from websockets.asyncio.client import connect

from kraken.exceptions import MaxReconnectError

Expand Down Expand Up @@ -127,10 +127,11 @@ async def __run(self: ConnectSpotWebsocketBase, event: asyncio.Event) -> None:
)
LOG.debug("Websocket token: %s", self.ws_conn_details)

async with websockets.connect( # pylint: disable=no-member
async with connect( # pylint: disable=no-member
f"wss://{self.__ws_endpoint}",
additional_headers={"User-Agent": "btschwertfeger/python-kraken-sdk"},
ping_interval=30,
max_queue=None, # FIXME: This is not recommended by the docs https://websockets.readthedocs.io/en/stable/reference/asyncio/client.html#module-websockets.asyncio.client
) as socket:
LOG.info("Websocket connected!")
self.socket = socket
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ requires-python = ">=3.11"
dependencies = [
"asyncio>=3.4",
"requests",
"websockets>=14",
"websockets>=14.1",
"click",
"cloup",
"orjson",
Expand Down
1 change: 1 addition & 0 deletions tests/spot/test_spot_base_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ async def check() -> None:

@pytest.mark.spot
@pytest.mark.spot_auth
@pytest.mark.timeout(120)
def test_spot_rest_async_client_post_report(
spot_api_key: str,
spot_secret_key: str,
Expand Down
13 changes: 9 additions & 4 deletions tests/spot/test_spot_orderbook.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,11 @@ def test_add_book(caplog: pytest.LogCaptureFixture) -> None:
async def execute_add_book() -> None:
async with SpotOrderBookClientWrapper() as orderbook:

await orderbook.add_book(pairs=["BTC/USD"])
await async_sleep(2)
# having multiple pairs to test the cancellation queue error absence
await orderbook.add_book(
pairs=["BTC/USD", "DOT/USD", "ETH/USD", "MATIC/USD", "BTC/EUR"],
)
await async_sleep(4)

book: dict | None = orderbook.get(pair="BTC/USD")
assert isinstance(book, dict)
Expand All @@ -154,8 +157,10 @@ async def execute_add_book() -> None:
asyncio.run(execute_add_book())

for expected in (
'{"method": "subscribe", "result": {"channel": "book", "depth": 10, "snapshot": true, "symbol": "BTC/USD"}, "success": true, "time_in": ',
'{"channel": "book", "type": "snapshot", "data": [{"symbol": "BTC/USD", "bids": ',
'{"method": "subscribe", "result": {"channel": "book", "depth": 10, '
'"snapshot": true, "symbol": "BTC/USD"}, "success": true, "time_in": ',
'{"channel": "book", "type": "snapshot", "data": '
'[{"symbol": "BTC/USD", "bids": ',
):
assert expected in caplog.text

Expand Down
1 change: 1 addition & 0 deletions tests/spot/test_spot_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ def test_get_trade_volume(spot_auth_user: User) -> None:
@pytest.mark.spot
@pytest.mark.spot_auth
@pytest.mark.spot_user
@pytest.mark.timeout(120)
def test_request_save_export_report(spot_auth_user: User) -> None:
"""
Checks the ``save_export_report`` function by requesting an
Expand Down