refactor(cli): move to apps implementation instead of deployments - #95
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:
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
Refactors the serverless CLI and internal API client to use the new “apps” resources/endpoints (replacing “deployments”), aligning CLI output, tests, and OpenAPI spec with the updated Serverless API surface.
Changes:
- Migrates CLI commands and output payloads from deployment-based naming/endpoints to app-based (
ListApps,GetApp,CreateApp, lifecycle, env vars, secrets). - Updates internal serverless API client methods/types and corresponding unit tests to use
/v1/apps/.... - Bumps and revises the Serverless OpenAPI spec to v0.4.0 (apps terminology, compute/gpu semantics, and related schema updates).
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/cmd/serverless/secrets.go | Switches secret list/attach/detach calls from deployment APIs to app APIs. |
| internal/cmd/serverless/pack.go | Updates comment terminology to “createApp”. |
| internal/cmd/serverless/open.go | Renames output fields from deploymentId to appId. |
| internal/cmd/serverless/display.go | Renames display wrappers and output structs to app-based fields (AppId, AppName, etc.). |
| internal/cmd/serverless/display_test.go | Updates table/output tests to app-based types/fields. |
| internal/cmd/serverless/deploy.go | Uses CreateApp/AppCreate/NewCodeAppSource; updates example and requires --gpu-type. |
| internal/cmd/serverless/apps.go | Switches listing/showing to ListApps/GetApp and updates sort/status parsing types. |
| internal/cmd/serverless/apps_scale.go | Switches scaling updates to UpdateApp/AppUpdate. |
| internal/cmd/serverless/apps_scale_test.go | Updates scale patch wrapper type to AppUpdate. |
| internal/cmd/serverless/apps_lifecycle.go | Switches stop/resume/delete lifecycle actions to app-based methods and return types. |
| internal/cmd/serverless/apps_env.go | Switches env var list/update/delete calls to app-based endpoints and output payloads. |
| internal/api/serverless/secrets.go | Renames secret attachment APIs to app-based (ListAppSecrets, AttachAppSecret, DetachAppSecret). |
| internal/api/serverless/secrets_test.go | Updates secrets client tests for /v1/apps/.../secrets routes and messaging. |
| internal/api/serverless/log_test.go | Updates redaction test JSON to use appId field. |
| internal/api/serverless/env.go | Renames env var APIs to app-based (ListAppEnvironmentVariables, etc.). |
| internal/api/serverless/env_test.go | Updates env var client tests for /v1/apps/.../environment-variables routes and payloads. |
| internal/api/serverless/client.go | Renames core client types/methods from deployments to apps; updates create timeout naming; updates code-source helper. |
| internal/api/serverless/client_test.go | Updates core client tests for apps routes/types and adds GPU type constant for create tests. |
| docs/runware_serverless_deploy.md | Updates deploy command examples/flag description to include required --gpu-type. |
| api/serverless/openapi.yaml | Updates API spec to v0.4.0: apps terminology, route changes, and schema/description updates. |
Suppressed comments (3)
internal/api/serverless/client_test.go:396
- This test uses
depas the variable name for a value returned byGetApp, which makes the assertions harder to read now that deployments have been renamed to apps.
dep, err := c.GetApp(context.Background(), testAppID)
internal/api/serverless/client_test.go:473
- The returned value from
UpdateAppis stored indep, which is leftover deployment naming. Renaming toappwill better reflect the API/resource under test.
dep, err := c.UpdateApp(context.Background(), testAppID, AppUpdate{
internal/api/serverless/client_test.go:583
- Within the lifecycle table test, the variable name
depis a leftover from the deployments terminology; usingappkeeps the test consistent with the new API naming.
dep, err := op.call(c, context.Background(), testAppID)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated no new comments.
Suppressed comments (1)
internal/api/serverless/client_test.go:137
- The CreateApp tests don’t currently assert that the request body includes the now-required
configuration.gpuType. This makes it easy for a future refactor to accidentally omit the field without test failures (the server handler always returns 201 regardless of request body). Consider decoding the request JSON in the handler and assertingconfiguration.gpuType == testGPUType(and optionallyappId).
func TestCreateApp(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost || r.URL.Path != "/v1/apps" {
t.Errorf("unexpected %s %s", r.Method, r.URL.Path)
}
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusCreated)
No description provided.