feat(serverless): initial command structure scaffolding - #79
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. To trigger a review, include ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR scaffolds an initial “serverless” command group in the Runware CLI, introducing a dedicated Serverless REST client, wiring the command into the root CLI, and generating initial reference docs.
Changes:
- Added Serverless API/Dashboard base URL configuration helpers and environment overrides.
- Introduced a new
internal/api/serverlessREST client and a first endpoint (ListGpuTypes) with tests. - Added a new
runware serverlesscommand group with initial subcommands (deploy,gpus,open) plus generated docs.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/config/config.go | Adds defaults + env overrides for Serverless API and dashboard URLs. |
| internal/cmdutil/browser.go | Adds a helper to open URLs in the default browser cross-platform. |
| internal/cmd/serverless/serverless.go | Registers the serverless command group and its subcommands. |
| internal/cmd/serverless/secret.go | Adds placeholder serverless apps secret command group. |
| internal/cmd/serverless/registry.go | Adds placeholder serverless registry command group. |
| internal/cmd/serverless/open.go | Adds serverless open command to open dashboard URLs and print the link. |
| internal/cmd/serverless/gpus.go | Adds serverless gpus command to fetch and render GPU catalogue + pricing. |
| internal/cmd/serverless/deploy.go | Adds serverless deploy command stub (currently unimplemented). |
| internal/cmd/serverless/apps.go | Adds placeholder serverless apps group and wires in secret. |
| internal/cmd/root.go | Wires serverless into the root CLI command tree. |
| internal/api/serverless/types.go | Defines Serverless GPU-related response types. |
| internal/api/serverless/gpu.go | Implements ListGpuTypes endpoint call. |
| internal/api/serverless/gpu_test.go | Adds tests for ListGpuTypes and error/auth behaviors. |
| internal/api/serverless/errors.go | Adds error parsing/mapping to shared transport.RunwareError. |
| internal/api/serverless/client.go | Adds a minimal REST client with auth + JSON decode + debug logging. |
| docs/runware.md | Adds runware serverless to the top-level command documentation. |
| docs/runware_serverless.md | Adds generated docs page for the serverless command group. |
| docs/runware_serverless_open.md | Adds generated docs page for serverless open. |
| docs/runware_serverless_gpus.md | Adds generated docs page for serverless gpus. |
| docs/runware_serverless_deploy.md | Adds generated docs page for serverless deploy. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // The launcher name is a fixed per-OS constant and the URL is passed as a | ||
| // separate argv element (not shell-interpreted), so there is no injection. | ||
| if err := exec.CommandContext(ctx, name, args...).Start(); err != nil { //nolint:gosec | ||
| return fmt.Errorf("failed to open browser: %w", err) | ||
| } | ||
| return nil |
| cmd.AddCommand( | ||
| newDeployCmd(), | ||
| newGPUsCmd(logger), | ||
| newOpenCmd(), | ||
| newRegistryCmd(), | ||
| newAppsCmd(), | ||
| ) |
| Args: cobra.MaximumNArgs(1), | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| return fmt.Errorf("not implemented yet") | ||
| }, |
danmrichards
left a comment
danmrichards
left a comment
There was a problem hiding this comment.
Lets also have this target a new rc/serverless or something similar. We don't want to have this in a production build just yet, so merging to main would prevent us pushing out any other fixes/features. Alternatively we'd need to have some build time killswitch for these commands.
There was a problem hiding this comment.
suggestion: lets not hand-roll a client for serverless, use the serverless OpenAPI spec and leverage a tool like https://github.com/oapi-codegen/oapi-codegen to generate a client instead
There was a problem hiding this comment.
suggestion: use the https://pkg.go.dev/github.com/pkg/browser package for this, it's well tested and has full cross-platform support
| Long: `Deploy a new serverless application from a Python file. | ||
|
|
||
| The application settings come from the project | ||
| configuration created by 'runware serverless init' or the dashboard.`, |
There was a problem hiding this comment.
question: I don't see any init command under serverless, coming later?
| Args: cobra.ExactArgs(1), | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| id := args[0] | ||
| url := strings.TrimSuffix(config.GetDashboardURL(), "/") + "/serverless/" + id |
There was a problem hiding this comment.
nitpick: escape the id param to avoid any weird concatenation
| url := strings.TrimSuffix(config.GetDashboardURL(), "/") + "/serverless/" + id | |
| url := strings.TrimSuffix(config.GetDashboardURL(), "/") + "/serverless/" + url.PathEscape(id) |
| client := serverlessapi.NewClient(config.GetAPIKey(), config.GetServerlessBaseURL(), slog.New(logger)) | ||
| gpus, err := client.ListGpuTypes(cmd.Context()) | ||
| if err != nil { | ||
| spin.Stop() |
There was a problem hiding this comment.
nitpick: both branches are stopping the spinner before any output, so should just be able to defer spin.Stop() after the spin.Start()
Scaffolding out the serverless product structure.