Skip to content

Tidy websocket code#797

Open
MetRonnie wants to merge 2 commits intocylc:masterfrom
MetRonnie:tidy
Open

Tidy websocket code#797
MetRonnie wants to merge 2 commits intocylc:masterfrom
MetRonnie:tidy

Conversation

@MetRonnie
Copy link
Member

Precursor to addressing #690, tidy up some of this code

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).
  • No dependency changes
  • Tests not needed
  • Changelog entry not needed as not user-facing
  • Code tidy on master branch

@MetRonnie MetRonnie added this to the 1.9.0 milestone Mar 12, 2026
@MetRonnie MetRonnie self-assigned this Mar 12, 2026
Comment on lines -103 to -106
try:
return self.ws.recv_nowait()
except WebSocketClosedError:
raise ConnectionClosedException()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function will never raise a WebSocketClosedError

def recv_nowait(self):
return self.queue.get_nowait()

Comment on lines -108 to -112
async def send(self, data):
if self.ws.ws_connection and not self.ws.ws_connection.is_closing():
await self.ws.write_message(data)
else:
raise WebSocketClosedError
Copy link
Member Author

@MetRonnie MetRonnie Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicates what self.ws.write_message(data) already does: https://github.com/tornadoweb/tornado/blob/fbc4b0332d068915e4b2593aea58d82532fac62c/tornado/websocket.py#L361-L365

Replaced this method with calling self.ws.write_message(data) directly

Comment on lines -118 to -121
async def close(self, code):
ws_close = self.ws.close(code)
if ws_close is not None:
await ws_close
Copy link
Member Author

@MetRonnie MetRonnie Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self.ws.close() is not async and always returns None: https://github.com/tornadoweb/tornado/blob/fbc4b0332d068915e4b2593aea58d82532fac62c/tornado/websocket.py#L496

Replaced this method with calling self.ws.close() directly

Comment on lines -85 to +84
self.pending_tasks = WeakSet()
self.pending_tasks: set[asyncio.Task] = set()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a WeakSet does not prevent garbage collection of the pending tasks, so this wasn't really doing anything as far as I can tell

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant