Add endpoint for the Dev UI to subscribe to runtime updates #1154
Conversation
| @@ -110,6 +117,14 @@ export class RuntimeManager { | |||
| ); | |||
| } | |||
|
|
|||
| subscribeToRuntimeEvents( | |||
There was a problem hiding this comment.
It would be manager.subscribeToRuntimeEvents(...) which makes it seem like the manager is subscribing. Maybe just onRuntimeEvents? Also please add a JSDoc explaining how to use it.
| @@ -79,6 +79,33 @@ export async function startServer(manager: RuntimeManager, port: number) { | |||
| res.end(); | |||
| }); | |||
|
|
|||
| app.get('/api/latest/runtime', async (_, res) => { | |||
There was a problem hiding this comment.
What is latest referring to here?
There was a problem hiding this comment.
Latest is referring to the fact that this endpoint will only ever return the most recent runtime info.
There was a problem hiding this comment.
Hmm, shouldn't it be flipped then? /api/runtimes/latest?
There was a problem hiding this comment.
Also any reason this shouldn't be part of the tRPC router?
There was a problem hiding this comment.
I ended up renaming this to /api/sse and made it general purpose for any events the server needs to send. I don't actually know that this will be a requirement... but... sse is limited to 6 connections for the entire browser (including tabs) under http(s). So better to set us up to have a single connection for the whole dev ui in the future.
As for why not tRPC, (a) my starting point is here (picking up from Shruti) #1053 but also (b) tRPC does not support SSE until v11, which is currently in "beta".
MichaelDoyle
force-pushed
the
sse-runtime-endpoint
branch
from
0e35b8e
to
3e54fbd
Compare
|
@apascal07 @tonybaroneee made some more tweaks based on feedback and my own testing. PTAL. |
Checklist (if applicable):
cc @tonybaroneee