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

config: configure default websocket ping interval #586

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

oliver-sanders
Copy link
Member

  • Closes websockets: configure default ping interval #557
  • Sends regular pings to websocket clients to ensure that the connection is still active and open at the other end.
  • This allows for earlier detection of client-side connection closing.
  • It should also circumnavigate web proxy killing of idle websockets.

To review, you might want to jam a debugger into this method:

https://github.com/tornadoweb/tornado/blob/f399f40fde0ae1b130646db783a6f79cc59231b2/tornado/websocket.py#L1540-L1557

Or mess around with the ping timeout.

Check List

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Applied any dependency changes to both setup.cfg (and conda-environment.yml if present).
  • Tests are included - tornado functionality vendored by jupyter_server, not something for us to test
  • CHANGES.md entry included if this is a change that can affect users
  • Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.
  • If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

* Closes cylc#557
* Sends regular pings to websocket clients to ensure that the connection
  is still active and open at the other end.
* This allows for earlier detection of client-side connection closing.
@oliver-sanders oliver-sanders added this to the 1.5.0 milestone Apr 23, 2024
@oliver-sanders oliver-sanders self-assigned this Apr 23, 2024
@oliver-sanders oliver-sanders marked this pull request as draft April 24, 2024 08:14
@oliver-sanders
Copy link
Member Author

This configuration appears to cause the connection to be closed with each ping, investigation required.

@oliver-sanders
Copy link
Member Author

It looks like there is an issue with the implementation of ping_timeout: tornadoweb/tornado#3258

Looks like an easy fix, having a crack.

@oliver-sanders
Copy link
Member Author

This will require an upstream fix: tornadoweb/tornado#3376

@oliver-sanders oliver-sanders modified the milestones: 1.5.0, 1.6.0 Apr 30, 2024
@oliver-sanders oliver-sanders added the blocked This work cannot proceed until other work has been completed. label Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked This work cannot proceed until other work has been completed. small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

websockets: configure default ping interval
1 participant