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

Implement JSON-RPC WebSocket server #93

Merged
merged 11 commits into from
Aug 20, 2024
Merged

Implement JSON-RPC WebSocket server #93

merged 11 commits into from
Aug 20, 2024

Conversation

samholmes
Copy link
Contributor

@samholmes samholmes commented Aug 8, 2024

Note to reviewer

A good amount of this PR is infra for supporting serverlet routes for HTTP endpoints. Some changes to serverlet's utilities could be brought upstream to accomplish the needs of this integration (lowering the line count for this PR).

In addition to using serverlets for the HTTP protocol, this PR demonstrates the use of serverlets for the WS and JSON-RPC protocols. It turns out with a small change to serverlet's types, it is a nice fit as an framework for building out the necessary middleware for WS and JSON-RPC support. Perhaps this could be accepted as a stand-alone serverlet middleware package for other upcoming WebSocket projects.

CHANGELOG

Does this branch warrant an entry to the CHANGELOG?

  • Yes
  • No

Dependencies

none

Description

none

@samholmes samholmes force-pushed the sam/websockets branch 2 times, most recently from 34b9770 to cdeae1f Compare August 8, 2024 22:09
Copy link
Contributor

@swansontec swansontec left a comment

Choose a reason for hiding this comment

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

I think we need a sidebar to discuss this architecture. This server only has 3 routes. It really doesn't need to be this complicated and abstracted.

Copy link
Contributor

@swansontec swansontec left a comment

Choose a reason for hiding this comment

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

The looks quite reasonable now!

@samholmes samholmes merged commit 2ac67f5 into master Aug 20, 2024
1 check passed
@paullinator paullinator deleted the sam/websockets branch September 13, 2024 23:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants