search

LUCIT-Systems-and-Development / unicorn-binance-websocket-api

A Python SDK by LUCIT to use the Binance Websocket API`s (com+testnet, com-margin+testnet, com-isolated_margin+testnet, com-futures+testnet, com-coin_futures, us, tr, dex/chain+testnet) in a simple, fast, flexible, robust and fully-featured way.
https://unicorn-binance-websocket-api.docs.lucit.tech/
Other
678 stars 166 forks source link

BinanceWebSocketApiManager.stop_stream() doesn't stop the stream immediately #161

Closed ArnoldWolfstein closed 2 years ago

ArnoldWolfstein commented 3 years ago

Check this or we will delete your issue. (fill in the checkbox with an X like so: [x])

Select one:

Environment

What kind of internet connection do you have?

cable

Average system load (CPU)

%10

Hardware specification

Operating System? (include version)

Options

Which endpoint do you connect?

 binance.com

Python Version Requirement

Exact Python Version?

Python 3.8.1

Pip Version?

pip 21.0.1

Dependencies

UNICORN Binance WebSocket API Version?

1.29.0

Description of your issue

stop_stream() doesn't stop the related stream immediately. As we can see the code just stops restart.

https://github.com/oliver-zehentleitner/unicorn-binance-websocket-api/blob/c975d49486f93761bbeb01e5a87ff9d4302129f6/unicorn_binance_websocket_api/unicorn_binance_websocket_api_manager.py#L3171

So we need to wait till next heartbeat/ping-pong. Maybe this is the design choice but the function name is a little bit confusing then. Maybe we rename it to: stop_restart_stream()? Or we can implement delete_listen_key_by_stream_id() as well as delete_stream_from_stream_list in it? Maybe both? Just a suggestion.

oliver-zehentleitner commented 3 years ago

No it does not just stops a restart. It also activates a stop request with self.stream_list[stream_id]['stop_request'] = True

ArnoldWolfstein commented 3 years ago

Yes, and when does the stream stop after activating?

oliver-zehentleitner commented 3 years ago

the stream loop in the socket class (own threads) is checking if there is a new request every round.

as mentioned here i want to try a new way to stop streams with loop.close().

oliver-zehentleitner commented 3 years ago

implementing delete_listen_key_by_stream_id() makes sense i guess!

delete_stream_from_stream_list() is not good, because it contains statistics that can help debugging and maybe devs want still have it available.We can implement a frequent_check that cleans the oldest inactive streams if theere are more 1000 entries on the stack.

ArnoldWolfstein commented 3 years ago

Yeah, makes sense: delete_listen_key_by_stream_id() should be enough.

oliver-zehentleitner commented 3 years ago

Just for documentation: https://github.com/oliver-zehentleitner/unicorn-binance-websocket-api/issues/137 depends on the same fix.

oliver-zehentleitner commented 2 years ago

I dont know whats the right or working way to solve this. Has someone an idea or a hint?

ebp-z commented 2 years ago

I dont know whats the right or working way to solve this. Has someone an idea or a hint?

since it awaits here until data is arrived or connection is closed: https://github.com/oliver-zehentleitner/unicorn-binance-websocket-api/blob/1e57327cf19bdacf280328f98ad5e5bffa7ae449/unicorn_binance_websocket_api/unicorn_binance_websocket_api_connection.py#L290

you can close websocket or use wait_for

oliver-zehentleitner commented 2 years ago

@ebp-z thanks for the hint. using wait_for() has led to restart of the socket after each timeout. i tested wit loop.run_until_complete() and loop.run_forever().

But i found out now, that the loop.stop() which i tryed in the past, is working with loop.run_forever() but not with loop.run_until_complete() what we used till now. i switched it and stop() works now :)

Thanks you!