NVIDIA / NVIDIA/OpenShell

feat(sdk/go): bundle Create() positional parameters into options struct

Open
#2,807 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

area:sdk area:sdk:go state:validated
Dominant language
Rust
Stars
8.7k
Forks
1.3k
Avg merge
2d 11h
Merged PRs (30d)
253

Description

Summary

SandboxInterface.Create() now takes 7 positional parameters plus variadic opts after the sandbox templates refactor in PR #2781:

Create(ctx context.Context, workspace, name string, workload *SandboxWorkloadConfig,
       policy *SandboxPolicy, providers []string, labels map[string]string,
       opts ...CreateOptions) (*Sandbox, error)

Four consecutive nilable parameters of different types makes call sites unreadable:

client.Sandboxes().Create(ctx, "default", "my-sandbox", nil, nil, nil, nil)

Proposed Change

Bundle the creation parameters into a CreateSandboxParams struct:

type CreateSandboxParams struct {
    Workload  *SandboxWorkloadConfig
    Policy    *SandboxPolicy
    Providers []string
    Labels    map[string]string
}

The interface becomes:

Create(ctx context.Context, workspace, name string, params CreateSandboxParams, opts ...CreateOptions) (*Sandbox, error)

This follows the k8s client-go pattern this SDK models, where creation parameters are bundled into typed structs rather than passed as positional arguments. Since PR #2781 is already a breaking change, the migration cost for this improvement is minimal.

The same pattern should apply to CreateFromTemplate() if it has a similar parameter list.

Context

Identified during PR #2781 code review (comment). The parameter list grew from 5 to 7+opts when policy and providers were promoted to top-level request fields.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the Go SDK definitions and implementations of SandboxInterface.Create(), then trace its call sites and tests. Check whether CreateFromTemplate() has a similar parameter list. Done means the typed CreateSandboxParams struct is used consistently, affected call sites are migrated, and the Go SDK tests pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
api
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.