-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Feat socket io #5056
Feat socket io #5056
Conversation
Still needs testing and integration of proper client (Rather than the mock I've been using)
Will this make #5019 easier to implement by decoupling the |
except: | ||
try: | ||
asyncio.get_running_loop() | ||
logger.warning( | ||
'error_reading_from_redis', exc_info=True, stack_info=True | ||
) | ||
except RuntimeError: | ||
return # Loop has been shut down |
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.
should we os.Exit
here or anything?
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.
No - if there are any other graceful shutdown tasks that might interfere with those - ending the loop should be sufficient.
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.
Only bit that'd be nice to tackle is that between SIGTERM and SIGKILL the agent appears as STOPPED
, even though it's still working during the grace period. We can tackle that in a follow-on though, this is working well as-is
Using SocketIO as the Underlying Communication Protocol
Migration / Refactor to use SocketIO as the communication protocol instead of raw Websockets
Testing Scenarios:
Starting a new session should open a single websocket


Refreshing the page should allow the session to continue uninterrupted (I refreshed the page then asked a follow on question):

Interrupting the socket without refreshing the page should allow the session to continue uninterrupted (You can see in the screenshot that I used the developer console to close the socket, and a new one instantly opened to replace it. I asked a follow up question with no issue):

Many of the Use cases for this PR (Particularly related to Redis are mostly applicable in the SAAS environment, where Pods may cease to exist mid session and we want to minimize user disruption. The logic when dealing with multiple pods is more complex than with a single pod!
Fixes #5151
To run this PR locally, use the following command: