-
Notifications
You must be signed in to change notification settings - Fork 301
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
Simplify monitor websocket use #986
Simplify monitor websocket use #986
Conversation
Codecov Report
@@ Coverage Diff @@
## master #986 +/- ##
========================================
- Coverage 79.12% 79% -0.13%
========================================
Files 45 45
Lines 6425 6431 +6
========================================
- Hits 5084 5081 -3
- Misses 1341 1350 +9 |
Only remaining issue is an annoying warning when reloading the monitor webpage. This seems to only happen with version 4.0.1 (and 4.0.0?) when reloading from Firefox (at least it doesn't seem to happen when reloading from chrome. As seen below the connection is correctly closed but then a new one is created that fails immediately with a loud warning. This is subsequently replaced by a new one again that works as expected.
|
I have been testing in context with the StationConfigurator it and it works excellently. |
There is still some problems:
IPython crashes.
The following does not crash
|
This ensures that the server is always stopped before the real join is called avoiding a deadlock on a running ws server
So it looks like join is called on the thread before either an atexit or a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed my issues.
@Dominik-Vogel I think this is ready to go |
So ensure it's awaited correctly. Also avoid calling is closed from outside the thread
@Dominik-Vogel I think the last issue is fixed now. wait_closed() is a co-routine that needs to be awaited. |
Author: Jens Hedegaard Nielsen <jenshnielsen@gmail.com> Simplify monitor websocket use (#986)
Should make it compatible with the latest release of websockets python package and fixes issues with restarting the monitor
@Dominik-Vogel