-
Notifications
You must be signed in to change notification settings - Fork 25
[Review] Reviewing #1570: Support for Support for SSE and NDJSON
#1805
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
Conversation
Support for SSE and NDJSON
|
This PR seems to contain a total re-work of the code, should I review this new implementation instead of using the one I did? |
f61ad03 to
e98915f
Compare
|
I updated the base to latest main, so now this needs to be rebased so I can evaluate the logic changes (if any). I'm not sure if the streaming support was actually tested here. The images provided do not show relays performing streaming requests. I think the approach of creating handlers per request ( I see lots of config changes but basically it seems that you are adding just two rules:
P.S.: I see the header timeout removed, great.. |
…1805) Reviewing `Streaming Support for SSE and NDJSON (used for LLMs)` in #1750. **Next steps**: - Outlined in this comment: #1570 (review) **Reproducing this locally (after running localnet)**: ```bash make send_relay_path_REST make pocketd_relayminer_relay_JSONRP ``` **Changes made**: - Updated the LocalNet toolchain to make it easier to test REST & LLM services - Added `make pocketd_relayminer_relay_JSONRPC` and revived `make send_relay_path_REST` - Reviewing streaming RelayMiner changes in [#1570](#1570) for code health & maintainbility - Moved logic from `sync.go` into `server.handleHttp` to follow a similar pattern as `server.handleHttpStream` --- The image below shows the result in LocalNet: <img width="3552" height="1527" alt="Screenshot 2025-09-30 at 8 10 22 PM" src="https://github.com/user-attachments/assets/bcf9b2dc-5560-4ab4-b06f-c1ebdfaaaa93" /> --- Co-authored-by: Ramiro Rodriguez Colmeiro <[email protected]>
Reviewing
Streaming Support for SSE and NDJSON (used for LLMs)in #1750.Next steps:
Reproducing this locally (after running localnet):
Changes made:
make pocketd_relayminer_relay_JSONRPCand revivedmake send_relay_path_RESTsync.gointoserver.handleHttpto follow a similar pattern asserver.handleHttpStreamThe image below shows the result in LocalNet: