feat: add --playground flag to cog serve - #3154
Conversation
Start the playground alongside the model on a dedicated port, print both URLs, and shut it down cleanly on Ctrl+C. The playground port never collides with the model port.
|
I reviewed the PR changes, focusing on the serve.go file to understand the --playground flag implementation. Based on my review, I found no actionable issues requiring comments. LGTM |
Use the actual serve host instead of hardcoding localhost, so the playground proxy matches the address the model is really bound to.
|
LGTM |
markphelps
left a comment
markphelps
left a comment
There was a problem hiding this comment.
Thanks for factoring the playground startup out cleanly. I found two edge cases that need fixing before this is ready.
| var playgroundSrv *http.Server | ||
| var playgroundLn net.Listener | ||
| if servePlaygroundFlag { | ||
| playgroundURL, playgroundSrv, playgroundLn, err = startPlayground(ctx, playgroundConfig{ |
There was a problem hiding this comment.
This makes async mode unusable for the integrated playground on Linux. playgroundHost is still the standalone command default (127.0.0.1), while the UI tells the model to send webhooks to host.docker.internal. The Linux host-gateway mapping is only added when --upload-url is set, and even with that mapping a container cannot reach a loopback-only listener through the gateway. Could the embedded playground bind on a container-reachable interface and add host.docker.internal:host-gateway whenever --playground is enabled? An end-to-end async webhook test here would catch this.
There was a problem hiding this comment.
The embedded playground now binds to 0.0.0.0 and adds the host-gateway alias whenever --playground is enabled.
| // port on overlapping interfaces. A playground port of 0 asks for a free port, | ||
| // so it never collides. | ||
| func validateServePorts(serveHost string, port, playgroundPort int, playgroundHost string) error { | ||
| if playgroundPort == 0 { |
There was a problem hiding this comment.
Port 0 does not guarantee that the playground avoids the model port: the playground listener is opened before Docker publishes the model port, so for a user-selected model port in the ephemeral range the OS can assign that exact port to the playground. Docker will then fail to start the model. After binding, please compare the resolved playground port with the model port (and retry or fail clearly if they overlap), rather than treating 0 as collision-free.
There was a problem hiding this comment.
The bound playground port is now checked before Docker starts, and Cog closes the listener and returns an error if it collides with the model port.
|
LGTM |
2 participants
Adds
--playgroundtocog serveso it starts the playground alongside the model and prints both URLs:The playground binds its own port (
--playground-port, default 9000,0= free port) and never collides with the model port.pkg/cli/serve.go: new--playgroundand--playground-portflags; validates the playground port doesn't overlap the model port; binds the playground before the container starts so a bind failure aborts fast; prints the playground URL next toServing at; runs it with graceful shutdown.cmdServenow cancels its context on signal, so Ctrl+C exits cleanly instead of erroring.pkg/cli/playground.go: pulls the standalone command's startup into a reusablestartPlayground(ctx, playgroundConfig)helper.cog playgroundis unchanged.Only
servesupports--playgroundfor now;predict/run/trainrun the container headlessly with no public port.