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

Refactoring manager for ease testing #305

Conversation

dima-dmytruk23
Copy link
Contributor

PR Details

I made a more flexible manager constructor so that you can replace the exchange with your own mock server.

Description

To replace the exchange with your mock server, just specify exchange=Exchanges.LOCALHOST, exchange_type='dex' (or cex) when initializing the manager. exchange_type must be specified only for Localhost. You can also set websocket_base_uri and max_subscriptions_per_stream otherwise the default values ​​will be applied.

Related Issue

#304

Motivation and Context

With the previous implementation - replacing the server with your own mock - was unreal.

How Has This Been Tested

Tested on localhost (MacOS) and in docker. I worked with a real exchange and with my own mock server.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the
    CONTRIBUTING
    document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@oliver-zehentleitner
Copy link
Member

oliver-zehentleitner commented Mar 2, 2023

Hello!

First of all thanks for the PR, the code looks very neat and valid.

Is it support for Apache MockServer? https://www.mock-server.com/

I didn't know that, I do similar with a SSH redirect: https://www.ssh.com/academy/ssh/tunneling-example

Could you provide more information for other users on how to use it with this lib?

What I noticed, REST is not sent to the proxy:
https://github.com/LUCIT-Systems-and-Development/unicorn-binance-websocket-api/blob/master/unicorn_binance_websocket_api/restclient.py

Thank you!

@dima-dmytruk23
Copy link
Contributor Author

Привет!

Прежде всего спасибо за пиар, код выглядит очень аккуратно и валидно.

Это поддержка Apache MockServer? https://www.mock-server.com/

Я этого не знал, я делаю то же самое с перенаправлением SSH: https://www.ssh.com/academy/ssh/tunneling-example

Не могли бы вы предоставить дополнительную информацию для других пользователей о том, как использовать ее с этой библиотекой?

Что я заметил, REST не отправляется на прокси: https://github.com/LUCIT-Systems-and-Development/unicorn-binance-websocket-api/blob/master/unicorn_binance_websocket_api/restclient.py

Спасибо!

@oliver-zehentleitner

Initially, I tried to raise JMeter, but there were difficulties, and I decided to write my own simple websocket server. For example, here it sends 1000 messages in ~1 second.

import asyncio
import json
import logging
import random
from datetime import datetime

import websockets
from binance import Client


logging.basicConfig(level=logging.INFO,
                    format="{asctime} {msecs} | {levelname} | {process} | {thread} | {module} | {filename}:{lineno} "
                           "| {message}",
                    style="{",
                    datefmt='%Y-%m-%d %H:%M:%S')
logger = logging.getLogger(__name__)


WS_HOST = 'localhost'
WS_PORT = 8765
MESSAGES_COUNT = 1000
MESSAGE_TIMING = 0.00063


def get_symbols():
    binance_client = Client()
    exchange_info = binance_client.futures_exchange_info()
    return [symbol_info['symbol'] for symbol_info in exchange_info['symbols']]


async def get_message():
    symbol = random.choice(symbols)
    return json.dumps({
        "data": {
            "e": "kline",
            "E": datetime.now().timestamp() * 1000000,
            "s": symbol,
            "k": {
                "t": 1638747660000,
                "T": 1638747719999,
                "s": symbol,
                "i": "1m",
                "f": 100,
                "L": 200,
                "o": "0.0010",
                "c": "0.0020",
                "h": "0.0025",
                "l": "0.0015",
                "v": "1000",
                "n": 100,
                "x": False,
                "q": "1.0000",
                "V": "500",
                "Q": "0.500",
                "B": "123456",
            }
        }
    })


async def handler(websocket, path):
    await websocket.recv()

    tm = datetime.now()
    for i in range(MESSAGES_COUNT):
        message = await get_message()
        await websocket.send(message)
        logger.info(f"{message=}")
        await asyncio.sleep(MESSAGE_TIMING)

    logger.info((datetime.now() - tm).total_seconds())
    return


def start_ws_server():
    start_server = websockets.serve(handler, WS_HOST, WS_PORT)
    asyncio.get_event_loop().run_until_complete(start_server)
    asyncio.get_event_loop().run_forever()


if __name__ == '__main__':
    symbols = get_symbols()
    start_ws_server()

@oliver-zehentleitner
Copy link
Member

Is it possible to add support for REST API in this package too?

This would be needed to run userData streams!

https://github.com/LUCIT-Systems-and-Development/unicorn-binance-websocket-api/blob/master/unicorn_binance_websocket_api/restclient.py

@dima-dmytruk23
Copy link
Contributor Author

Is it possible to add support for REST API in this package too?

This would be needed to run userData streams!

https://github.com/LUCIT-Systems-and-Development/unicorn-binance-websocket-api/blob/master/unicorn_binance_websocket_api/restclient.py

Yes, sure. You can do it.

I just limited myself to web sockets in my task. The REST did not look. If you have the desire and opportunity - you can change the PR and tag me. I won't have time next week.

@oliver-zehentleitner
Copy link
Member

I am also full of tasks :/ i started preparing a new update for full UBS packages...

I would appreciate if we could add this to the next release but from my point of view we have to do:

  • Documenation: what is this implementation, how can it be used
  • Unittests: Thanks for the script above, this should help to test. I general we can apply a new kind of testing now because we can test against our self via localhost...
  • Adding REST support, i dont want merge half implementations...

I can do unittests and docs, because i want to learn and understand both.

Could you do the REST implementation?

@dima-dmytruk23
Copy link
Contributor Author

I am also full of tasks :/ i started preparing a new update for full UBS packages...

I would appreciate if we could add this to the next release but from my point of view we have to do:

  • Documenation: what is this implementation, how can it be used
  • Unittests: Thanks for the script above, this should help to test. I general we can apply a new kind of testing now because we can test against our self via localhost...
  • Adding REST support, i dont want merge half implementations...

I can do unittests and docs, because i want to learn and understand both.

Could you do the REST implementation?

Ok. Let me try on the weekend to implement for REST. Will you do documentation and tests?

@oliver-zehentleitner
Copy link
Member

deal! i will do my part afterwards when this is merged.

@oliver-zehentleitner
Copy link
Member

oliver-zehentleitner commented Mar 6, 2023

f"{self.exchange_type=}"

Awesome, did not know that :)

Is it only possible to use localhost with the hardcoded port or can this be overwritten?

@oliver-zehentleitner oliver-zehentleitner self-assigned this Mar 6, 2023
@oliver-zehentleitner oliver-zehentleitner added the enhancement New feature or request label Mar 6, 2023
@dima-dmytruk23
Copy link
Contributor Author

dima-dmytruk23 commented Mar 7, 2023

f"{self.exchange_type=}"

Awesome, did not know that :)

Yeah. It's from python3.8

Is it only possible to use localhost with the hardcoded port or can this be overwritten?

Hardcoded. To flexibly set the port, you need to rewrite it here (separate the key with the URI separately into host and port (tuple/dictionary)) and process it in the constructor of manager.

https://github.com/LUCIT-Systems-and-Development/unicorn-binance-websocket-api/pull/305/files#diff-540fe6a75df813cec7157aacac6de4268fd66a84f6bd3b2daaf7310f4202277eR43

@oliver-zehentleitner
Copy link
Member

Thanks , I will try when this PR is ready

@oliver-zehentleitner
Copy link
Member

I will merge it, we also add socks server support and your code is usefull. thanks!

@oliver-zehentleitner oliver-zehentleitner merged commit a757a0e into LUCIT-Systems-and-Development:master Mar 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants