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

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

Closed
5 of 13 tasks
ArnoldWolfstein opened this issue Apr 6, 2021 · 9 comments
Closed
5 of 13 tasks
Assignees
Labels
bug Something isn't working

Comments

@ArnoldWolfstein
Copy link

ArnoldWolfstein commented Apr 6, 2021

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

  • I have searched for other issues with the same problem or similar feature requests.

Select one:

  • Bug
  • Feature Request
  • Technical Help
  • Other

Environment

  • Are you using the module on a VPS or other Cloud hosting?
  • Are you using the module on a Raspberry Pi?

What kind of internet connection do you have?

cable

Average system load (CPU)

%10

Hardware specification

Operating System? (include version)

  • macOS
  • Windows
  • Linux (include flavour)

Options

  • stream_buffer
  • process_stream_data

Which endpoint do you connect?

 binance.com

Python Version Requirement

  • I am using Python 3.6.1 or above

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.

@ArnoldWolfstein ArnoldWolfstein added the bug Something isn't working label Apr 6, 2021
@ArnoldWolfstein ArnoldWolfstein changed the title BinanceWebSocketApiManager.stop_stream() doensn't stop the stream immediately BinanceWebSocketApiManager.stop_stream() doesn't stop the stream immediately Apr 6, 2021
@oliver-zehentleitner
Copy link
Member

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

@ArnoldWolfstein
Copy link
Author

ArnoldWolfstein commented Apr 7, 2021

Yes, and when does the stream stop after activating?

@oliver-zehentleitner
Copy link
Member

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
Copy link
Member

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
Copy link
Author

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

@oliver-zehentleitner
Copy link
Member

Just for documentation: #137 depends on the same fix.

@oliver-zehentleitner
Copy link
Member

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

@ebp-z
Copy link

ebp-z commented Oct 28, 2021

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 oliver-zehentleitner removed this from In progress in Todo Jan 7, 2022
@oliver-zehentleitner
Copy link
Member

@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!

BestCryptoKnight pushed a commit to BestCryptoKnight/unicorn-binance-websocket-api that referenced this issue Jul 10, 2022
BestCryptoKnight pushed a commit to BestCryptoKnight/unicorn-binance-websocket-api that referenced this issue Jul 10, 2022
Seven-112 pushed a commit to Seven-112/unicorn-binance-websocket-api that referenced this issue May 16, 2023
Seven-112 pushed a commit to Seven-112/unicorn-binance-websocket-api that referenced this issue May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants