From 6b1876161d696629bdf8e2e0d8c34951313fb8c9 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Fri, 1 Oct 2021 12:53:35 -0400 Subject: [PATCH 1/5] Update DeleteCodespaces to new API endpoint - Drop the need for the user argument - Update mocks - Remove no longer applicable TODO comment - Show message for successful deletion (this regressed) --- internal/codespaces/api/api.go | 12 +++--------- pkg/cmd/codespace/common.go | 2 +- pkg/cmd/codespace/delete.go | 10 ++++++++-- pkg/cmd/codespace/delete_test.go | 5 +---- pkg/cmd/codespace/mock_api.go | 14 ++++---------- 5 files changed, 17 insertions(+), 26 deletions(-) diff --git a/internal/codespaces/api/api.go b/internal/codespaces/api/api.go index 7ab3867aa..ea2e8fcaa 100644 --- a/internal/codespaces/api/api.go +++ b/internal/codespaces/api/api.go @@ -520,19 +520,13 @@ func (a *API) startCreate(ctx context.Context, repoID int, machine, branch, loca return &response, nil } -func (a *API) DeleteCodespace(ctx context.Context, user string, codespaceName string) error { - token, err := a.GetCodespaceToken(ctx, user, codespaceName) - if err != nil { - return fmt.Errorf("error getting codespace token: %w", err) - } - - req, err := http.NewRequest(http.MethodDelete, a.githubAPI+"/vscs_internal/user/"+user+"/codespaces/"+codespaceName, nil) +func (a *API) DeleteCodespace(ctx context.Context, codespaceName string) error { + req, err := http.NewRequest(http.MethodDelete, a.githubAPI+"/user/codespaces/"+codespaceName, nil) if err != nil { return fmt.Errorf("error creating request: %w", err) } - // TODO: use a.setHeaders() - req.Header.Set("Authorization", "Bearer "+token) + a.setHeaders(req) resp, err := a.do(ctx, req, "/vscs_internal/user/*/codespaces/*") if err != nil { return fmt.Errorf("error making request: %w", err) diff --git a/pkg/cmd/codespace/common.go b/pkg/cmd/codespace/common.go index ecce47b2a..bc2b04bbc 100644 --- a/pkg/cmd/codespace/common.go +++ b/pkg/cmd/codespace/common.go @@ -36,7 +36,7 @@ type apiClient interface { GetCodespaceToken(ctx context.Context, user, name string) (string, error) GetCodespace(ctx context.Context, token, user, name string) (*api.Codespace, error) ListCodespaces(ctx context.Context) ([]*api.Codespace, error) - DeleteCodespace(ctx context.Context, user, name string) error + DeleteCodespace(ctx context.Context, name string) error StartCodespace(ctx context.Context, token string, codespace *api.Codespace) error CreateCodespace(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) GetRepository(ctx context.Context, nwo string) (*api.Repository, error) diff --git a/pkg/cmd/codespace/delete.go b/pkg/cmd/codespace/delete.go index 8ea821dc9..3cdea3e20 100644 --- a/pkg/cmd/codespace/delete.go +++ b/pkg/cmd/codespace/delete.go @@ -80,7 +80,6 @@ func (a *App) Delete(ctx context.Context, opts deleteOptions) error { nameFilter = c.Name } } else { - // TODO: this token is discarded and then re-requested later in DeleteCodespace token, err := a.apiClient.GetCodespaceToken(ctx, user.Login, nameFilter) if err != nil { return fmt.Errorf("error getting codespace token: %w", err) @@ -132,7 +131,7 @@ func (a *App) Delete(ctx context.Context, opts deleteOptions) error { for _, c := range codespacesToDelete { codespaceName := c.Name g.Go(func() error { - if err := a.apiClient.DeleteCodespace(ctx, user.Login, codespaceName); err != nil { + if err := a.apiClient.DeleteCodespace(ctx, codespaceName); err != nil { _, _ = a.logger.Errorf("error deleting codespace %q: %v\n", codespaceName, err) return err } @@ -143,6 +142,13 @@ func (a *App) Delete(ctx context.Context, opts deleteOptions) error { if err := g.Wait(); err != nil { return errors.New("some codespaces failed to delete") } + + noun := "Codespace" + if len(codespacesToDelete) > 1 { + noun = noun + "s" + } + a.logger.Println(noun + " deleted.") + return nil } diff --git a/pkg/cmd/codespace/delete_test.go b/pkg/cmd/codespace/delete_test.go index bf16f82f0..1396c0c09 100644 --- a/pkg/cmd/codespace/delete_test.go +++ b/pkg/cmd/codespace/delete_test.go @@ -156,10 +156,7 @@ func TestDelete(t *testing.T) { GetUserFunc: func(_ context.Context) (*api.User, error) { return user, nil }, - DeleteCodespaceFunc: func(_ context.Context, userLogin, name string) error { - if userLogin != user.Login { - return fmt.Errorf("unexpected user %q", userLogin) - } + DeleteCodespaceFunc: func(_ context.Context, name string) error { if tt.deleteErr != nil { return tt.deleteErr } diff --git a/pkg/cmd/codespace/mock_api.go b/pkg/cmd/codespace/mock_api.go index 669083a32..b4edf6cdd 100644 --- a/pkg/cmd/codespace/mock_api.go +++ b/pkg/cmd/codespace/mock_api.go @@ -22,7 +22,7 @@ import ( // CreateCodespaceFunc: func(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) { // panic("mock out the CreateCodespace method") // }, -// DeleteCodespaceFunc: func(ctx context.Context, user string, name string) error { +// DeleteCodespaceFunc: func(ctx context.Context, name string) error { // panic("mock out the DeleteCodespace method") // }, // GetCodespaceFunc: func(ctx context.Context, token string, user string, name string) (*api.Codespace, error) { @@ -66,7 +66,7 @@ type apiClientMock struct { CreateCodespaceFunc func(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) // DeleteCodespaceFunc mocks the DeleteCodespace method. - DeleteCodespaceFunc func(ctx context.Context, user string, name string) error + DeleteCodespaceFunc func(ctx context.Context, name string) error // GetCodespaceFunc mocks the GetCodespace method. GetCodespaceFunc func(ctx context.Context, token string, user string, name string) (*api.Codespace, error) @@ -115,8 +115,6 @@ type apiClientMock struct { DeleteCodespace []struct { // Ctx is the ctx argument value. Ctx context.Context - // User is the user argument value. - User string // Name is the name argument value. Name string } @@ -279,23 +277,21 @@ func (mock *apiClientMock) CreateCodespaceCalls() []struct { } // DeleteCodespace calls DeleteCodespaceFunc. -func (mock *apiClientMock) DeleteCodespace(ctx context.Context, user string, name string) error { +func (mock *apiClientMock) DeleteCodespace(ctx context.Context, name string) error { if mock.DeleteCodespaceFunc == nil { panic("apiClientMock.DeleteCodespaceFunc: method is nil but apiClient.DeleteCodespace was just called") } callInfo := struct { Ctx context.Context - User string Name string }{ Ctx: ctx, - User: user, Name: name, } mock.lockDeleteCodespace.Lock() mock.calls.DeleteCodespace = append(mock.calls.DeleteCodespace, callInfo) mock.lockDeleteCodespace.Unlock() - return mock.DeleteCodespaceFunc(ctx, user, name) + return mock.DeleteCodespaceFunc(ctx, name) } // DeleteCodespaceCalls gets all the calls that were made to DeleteCodespace. @@ -303,12 +299,10 @@ func (mock *apiClientMock) DeleteCodespace(ctx context.Context, user string, nam // len(mockedapiClient.DeleteCodespaceCalls()) func (mock *apiClientMock) DeleteCodespaceCalls() []struct { Ctx context.Context - User string Name string } { var calls []struct { Ctx context.Context - User string Name string } mock.lockDeleteCodespace.RLock() From 61af29bb968ad77a654f3db68ae67c21e949a238 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Fri, 1 Oct 2021 13:02:29 -0400 Subject: [PATCH 2/5] Update telemetry path --- internal/codespaces/api/api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/codespaces/api/api.go b/internal/codespaces/api/api.go index ea2e8fcaa..d8d043e6a 100644 --- a/internal/codespaces/api/api.go +++ b/internal/codespaces/api/api.go @@ -527,7 +527,7 @@ func (a *API) DeleteCodespace(ctx context.Context, codespaceName string) error { } a.setHeaders(req) - resp, err := a.do(ctx, req, "/vscs_internal/user/*/codespaces/*") + resp, err := a.do(ctx, req, "/user/codespaces/*") if err != nil { return fmt.Errorf("error making request: %w", err) } From a3efb53c443d07dfab92a2dae7d66073d6e47124 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Mon, 4 Oct 2021 08:32:02 -0400 Subject: [PATCH 3/5] Update API.StartCodespace to use new API endpoint - Switch to using name instead of GUID - Remove GUID from the code since it is not used anywhere else - Add docs to the api client methods - Re-gen mocked client --- internal/codespaces/api/api.go | 44 ++++++++++++++++++++++--------- internal/codespaces/codespaces.go | 4 +-- pkg/cmd/codespace/common.go | 2 +- pkg/cmd/codespace/mock_api.go | 34 ++++++++++-------------- 4 files changed, 49 insertions(+), 35 deletions(-) diff --git a/internal/codespaces/api/api.go b/internal/codespaces/api/api.go index ea2e8fcaa..6305d6f06 100644 --- a/internal/codespaces/api/api.go +++ b/internal/codespaces/api/api.go @@ -43,6 +43,7 @@ import ( const githubAPI = "https://api.github.com" +// API is the interface to the codespace service. type API struct { token string client httpClient @@ -53,6 +54,7 @@ type httpClient interface { Do(req *http.Request) (*http.Response, error) } +// New creates a new API client with the given token and HTTP client. func New(token string, httpClient httpClient) *API { return &API{ token: token, @@ -61,10 +63,12 @@ func New(token string, httpClient httpClient) *API { } } +// User represents a GitHub user. type User struct { Login string `json:"login"` } +// GetUser returns the user associated with the given token. func (a *API) GetUser(ctx context.Context) (*User, error) { req, err := http.NewRequest(http.MethodGet, a.githubAPI+"/user", nil) if err != nil { @@ -95,6 +99,7 @@ func (a *API) GetUser(ctx context.Context) (*User, error) { return &response, nil } +// jsonErrorResponse returns the error message from a JSON response. func jsonErrorResponse(b []byte) error { var response struct { Message string `json:"message"` @@ -106,10 +111,12 @@ func jsonErrorResponse(b []byte) error { return errors.New(response.Message) } +// Repository represents a GitHub repository. type Repository struct { ID int `json:"id"` } +// GetRepository returns the repository associated with the given owner and name. func (a *API) GetRepository(ctx context.Context, nwo string) (*Repository, error) { req, err := http.NewRequest(http.MethodGet, a.githubAPI+"/repos/"+strings.ToLower(nwo), nil) if err != nil { @@ -140,9 +147,9 @@ func (a *API) GetRepository(ctx context.Context, nwo string) (*Repository, error return &response, nil } +// Codespace represents a codespace. type Codespace struct { Name string `json:"name"` - GUID string `json:"guid"` CreatedAt string `json:"created_at"` LastUsedAt string `json:"last_used_at"` Branch string `json:"branch"` @@ -168,6 +175,7 @@ type CodespaceEnvironmentGitStatus struct { } const ( + // CodespaceEnvironmentStateAvailable is the state for a running codespace environment. CodespaceEnvironmentStateAvailable = "Available" ) @@ -179,6 +187,7 @@ type CodespaceEnvironmentConnection struct { HostPublicKeys []string `json:"hostPublicKeys"` } +// ListCodespaces returns a list of codespaces for the user. func (a *API) ListCodespaces(ctx context.Context) ([]*Codespace, error) { req, err := http.NewRequest( http.MethodGet, a.githubAPI+"/user/codespaces", nil, @@ -212,6 +221,7 @@ func (a *API) ListCodespaces(ctx context.Context) ([]*Codespace, error) { return response.Codespaces, nil } +// getCodespaceTokenRequest is the request body for the get codespace token endpoint. type getCodespaceTokenRequest struct { MintRepositoryToken bool `json:"mint_repository_token"` } @@ -224,6 +234,7 @@ type getCodespaceTokenResponse struct { // creation of a codespace is not yet complete and that the caller should try again. var ErrNotProvisioned = errors.New("codespace not provisioned") +// GetCodespaceToken returns a codespace token for the user. func (a *API) GetCodespaceToken(ctx context.Context, ownerLogin, codespaceName string) (string, error) { reqBody, err := json.Marshal(getCodespaceTokenRequest{true}) if err != nil { @@ -267,6 +278,7 @@ func (a *API) GetCodespaceToken(ctx context.Context, ownerLogin, codespaceName s return response.RepositoryToken, nil } +// GetCodespace returns a codespace for the user. func (a *API) GetCodespace(ctx context.Context, token, owner, codespace string) (*Codespace, error) { req, err := http.NewRequest( http.MethodGet, @@ -302,19 +314,20 @@ func (a *API) GetCodespace(ctx context.Context, token, owner, codespace string) return &response, nil } -func (a *API) StartCodespace(ctx context.Context, token string, codespace *Codespace) error { +// StartCodespace starts a codespace for the user. +// If the codespace is already running, the returned error from the API is ignored. +func (a *API) StartCodespace(ctx context.Context, codespaceName string) error { req, err := http.NewRequest( http.MethodPost, - a.githubAPI+"/vscs_internal/proxy/environments/"+codespace.GUID+"/start", + a.githubAPI+"/user/codespaces/"+codespaceName+"/start", nil, ) if err != nil { return fmt.Errorf("error creating request: %w", err) } - // TODO: use a.setHeaders() - req.Header.Set("Authorization", "Bearer "+token) - resp, err := a.do(ctx, req, "/vscs_internal/proxy/environments/*/start") + a.setHeaders(req) + resp, err := a.do(ctx, req, "/user/codespaces/*/start") if err != nil { return fmt.Errorf("error making request: %w", err) } @@ -326,19 +339,20 @@ func (a *API) StartCodespace(ctx context.Context, token string, codespace *Codes } if resp.StatusCode != http.StatusOK { + if resp.StatusCode == http.StatusConflict { + // 409 means the codespace is already running which we can safely ignore + return nil + } + // Error response may be a numeric code or a JSON {"message": "..."}. if bytes.HasPrefix(b, []byte("{")) { return jsonErrorResponse(b) // probably JSON } + if len(b) > 100 { b = append(b[:97], "..."...) } - if strings.TrimSpace(string(b)) == "7" { - // Non-HTTP 200 with error code 7 (EnvironmentNotShutdown) is benign. - // Ignore it. - } else { - return fmt.Errorf("failed to start codespace: %s", b) - } + return fmt.Errorf("failed to start codespace: %s", b) } return nil @@ -348,6 +362,7 @@ type getCodespaceRegionLocationResponse struct { Current string `json:"current"` } +// GetCodespaceRegionLocation returns the closest codespace location for the user. func (a *API) GetCodespaceRegionLocation(ctx context.Context) (string, error) { req, err := http.NewRequest(http.MethodGet, "https://online.visualstudio.com/api/v1/locations", nil) if err != nil { @@ -382,6 +397,7 @@ type SKU struct { DisplayName string `json:"display_name"` } +// GetCodespacesSKUs returns the available SKUs for the user for a given repo, branch and location. func (a *API) GetCodespacesSKUs(ctx context.Context, user *User, repository *Repository, branch, location string) ([]*SKU, error) { req, err := http.NewRequest(http.MethodGet, a.githubAPI+"/vscs_internal/user/"+user.Login+"/skus", nil) if err != nil { @@ -520,6 +536,7 @@ func (a *API) startCreate(ctx context.Context, repoID int, machine, branch, loca return &response, nil } +// DeleteCodespace deletes the given codespace. func (a *API) DeleteCodespace(ctx context.Context, codespaceName string) error { req, err := http.NewRequest(http.MethodDelete, a.githubAPI+"/user/codespaces/"+codespaceName, nil) if err != nil { @@ -616,6 +633,8 @@ func (a *API) AuthorizedKeys(ctx context.Context, user string) ([]byte, error) { return b, nil } +// do executes the given request and returns the response. It creates an +// opentracing span to track the length of the request. func (a *API) do(ctx context.Context, req *http.Request, spanName string) (*http.Response, error) { // TODO(adonovan): use NewRequestWithContext(ctx) and drop ctx parameter. span, ctx := opentracing.StartSpanFromContext(ctx, spanName) @@ -624,6 +643,7 @@ func (a *API) do(ctx context.Context, req *http.Request, spanName string) (*http return a.client.Do(req) } +// setHeaders sets the required headers for the API. func (a *API) setHeaders(req *http.Request) { if a.token != "" { req.Header.Set("Authorization", "Bearer "+a.token) diff --git a/internal/codespaces/codespaces.go b/internal/codespaces/codespaces.go index 654504205..968217280 100644 --- a/internal/codespaces/codespaces.go +++ b/internal/codespaces/codespaces.go @@ -26,7 +26,7 @@ func connectionReady(codespace *api.Codespace) bool { type apiClient interface { GetCodespace(ctx context.Context, token, user, name string) (*api.Codespace, error) GetCodespaceToken(ctx context.Context, user, codespace string) (string, error) - StartCodespace(ctx context.Context, token string, codespace *api.Codespace) error + StartCodespace(ctx context.Context, name string) error } // ConnectToLiveshare waits for a Codespace to become running, @@ -36,7 +36,7 @@ func ConnectToLiveshare(ctx context.Context, log logger, apiClient apiClient, us if codespace.Environment.State != api.CodespaceEnvironmentStateAvailable { startedCodespace = true log.Print("Starting your codespace...") - if err := apiClient.StartCodespace(ctx, token, codespace); err != nil { + if err := apiClient.StartCodespace(ctx, codespace.Name); err != nil { return nil, fmt.Errorf("error starting codespace: %w", err) } } diff --git a/pkg/cmd/codespace/common.go b/pkg/cmd/codespace/common.go index bc2b04bbc..c43e618f9 100644 --- a/pkg/cmd/codespace/common.go +++ b/pkg/cmd/codespace/common.go @@ -37,7 +37,7 @@ type apiClient interface { GetCodespace(ctx context.Context, token, user, name string) (*api.Codespace, error) ListCodespaces(ctx context.Context) ([]*api.Codespace, error) DeleteCodespace(ctx context.Context, name string) error - StartCodespace(ctx context.Context, token string, codespace *api.Codespace) error + StartCodespace(ctx context.Context, name string) error CreateCodespace(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) GetRepository(ctx context.Context, nwo string) (*api.Repository, error) AuthorizedKeys(ctx context.Context, user string) ([]byte, error) diff --git a/pkg/cmd/codespace/mock_api.go b/pkg/cmd/codespace/mock_api.go index b4edf6cdd..bda277e34 100644 --- a/pkg/cmd/codespace/mock_api.go +++ b/pkg/cmd/codespace/mock_api.go @@ -49,7 +49,7 @@ import ( // ListCodespacesFunc: func(ctx context.Context) ([]*api.Codespace, error) { // panic("mock out the ListCodespaces method") // }, -// StartCodespaceFunc: func(ctx context.Context, token string, codespace *api.Codespace) error { +// StartCodespaceFunc: func(ctx context.Context, name string) error { // panic("mock out the StartCodespace method") // }, // } @@ -93,7 +93,7 @@ type apiClientMock struct { ListCodespacesFunc func(ctx context.Context) ([]*api.Codespace, error) // StartCodespaceFunc mocks the StartCodespace method. - StartCodespaceFunc func(ctx context.Context, token string, codespace *api.Codespace) error + StartCodespaceFunc func(ctx context.Context, name string) error // calls tracks calls to the methods. calls struct { @@ -186,10 +186,8 @@ type apiClientMock struct { StartCodespace []struct { // Ctx is the ctx argument value. Ctx context.Context - // Token is the token argument value. - Token string - // Codespace is the codespace argument value. - Codespace *api.Codespace + // Name is the name argument value. + Name string } } lockAuthorizedKeys sync.RWMutex @@ -608,37 +606,33 @@ func (mock *apiClientMock) ListCodespacesCalls() []struct { } // StartCodespace calls StartCodespaceFunc. -func (mock *apiClientMock) StartCodespace(ctx context.Context, token string, codespace *api.Codespace) error { +func (mock *apiClientMock) StartCodespace(ctx context.Context, name string) error { if mock.StartCodespaceFunc == nil { panic("apiClientMock.StartCodespaceFunc: method is nil but apiClient.StartCodespace was just called") } callInfo := struct { - Ctx context.Context - Token string - Codespace *api.Codespace + Ctx context.Context + Name string }{ - Ctx: ctx, - Token: token, - Codespace: codespace, + Ctx: ctx, + Name: name, } mock.lockStartCodespace.Lock() mock.calls.StartCodespace = append(mock.calls.StartCodespace, callInfo) mock.lockStartCodespace.Unlock() - return mock.StartCodespaceFunc(ctx, token, codespace) + return mock.StartCodespaceFunc(ctx, name) } // StartCodespaceCalls gets all the calls that were made to StartCodespace. // Check the length with: // len(mockedapiClient.StartCodespaceCalls()) func (mock *apiClientMock) StartCodespaceCalls() []struct { - Ctx context.Context - Token string - Codespace *api.Codespace + Ctx context.Context + Name string } { var calls []struct { - Ctx context.Context - Token string - Codespace *api.Codespace + Ctx context.Context + Name string } mock.lockStartCodespace.RLock() calls = mock.calls.StartCodespace From e0db10e4dd72c02e1c3b820661c50a766d7defd8 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Mon, 4 Oct 2021 13:40:18 -0400 Subject: [PATCH 4/5] Switch API.GetCodespacesMachines to use new API - The SKU terminology is also dropped in favor of "machine" which matches the nomenclature of the rest of the product. --- internal/codespaces/api/api.go | 17 ++++--- pkg/cmd/codespace/common.go | 2 +- pkg/cmd/codespace/create.go | 58 +++++++++++------------ pkg/cmd/codespace/mock_api.go | 84 ++++++++++++++++------------------ 4 files changed, 77 insertions(+), 84 deletions(-) diff --git a/internal/codespaces/api/api.go b/internal/codespaces/api/api.go index 6305d6f06..148bcc4c9 100644 --- a/internal/codespaces/api/api.go +++ b/internal/codespaces/api/api.go @@ -34,7 +34,6 @@ import ( "fmt" "io/ioutil" "net/http" - "strconv" "strings" "time" @@ -392,14 +391,15 @@ func (a *API) GetCodespaceRegionLocation(ctx context.Context) (string, error) { return response.Current, nil } -type SKU struct { +type Machine struct { Name string `json:"name"` DisplayName string `json:"display_name"` } -// GetCodespacesSKUs returns the available SKUs for the user for a given repo, branch and location. -func (a *API) GetCodespacesSKUs(ctx context.Context, user *User, repository *Repository, branch, location string) ([]*SKU, error) { - req, err := http.NewRequest(http.MethodGet, a.githubAPI+"/vscs_internal/user/"+user.Login+"/skus", nil) +// GetCodespacesMachines returns the codespaces machines for the given repo, branch and location. +func (a *API) GetCodespacesMachines(ctx context.Context, repoID int, branch, location string) ([]*Machine, error) { + reqURL := fmt.Sprintf("%s/repositories/%d/codespaces/machines", a.githubAPI, repoID) + req, err := http.NewRequest(http.MethodGet, reqURL, nil) if err != nil { return nil, fmt.Errorf("error creating request: %w", err) } @@ -407,11 +407,10 @@ func (a *API) GetCodespacesSKUs(ctx context.Context, user *User, repository *Rep q := req.URL.Query() q.Add("location", location) q.Add("ref", branch) - q.Add("repository_id", strconv.Itoa(repository.ID)) req.URL.RawQuery = q.Encode() a.setHeaders(req) - resp, err := a.do(ctx, req, "/vscs_internal/user/*/skus") + resp, err := a.do(ctx, req, "/repositories/*/codespaces/machines") if err != nil { return nil, fmt.Errorf("error making request: %w", err) } @@ -427,13 +426,13 @@ func (a *API) GetCodespacesSKUs(ctx context.Context, user *User, repository *Rep } var response struct { - SKUs []*SKU `json:"skus"` + Machines []*Machine `json:"machines"` } if err := json.Unmarshal(b, &response); err != nil { return nil, fmt.Errorf("error unmarshaling response: %w", err) } - return response.SKUs, nil + return response.Machines, nil } // CreateCodespaceParams are the required parameters for provisioning a Codespace. diff --git a/pkg/cmd/codespace/common.go b/pkg/cmd/codespace/common.go index c43e618f9..3507d00af 100644 --- a/pkg/cmd/codespace/common.go +++ b/pkg/cmd/codespace/common.go @@ -42,7 +42,7 @@ type apiClient interface { GetRepository(ctx context.Context, nwo string) (*api.Repository, error) AuthorizedKeys(ctx context.Context, user string) ([]byte, error) GetCodespaceRegionLocation(ctx context.Context) (string, error) - GetCodespacesSKUs(ctx context.Context, user *api.User, repository *api.Repository, branch, location string) ([]*api.SKU, error) + GetCodespacesMachines(ctx context.Context, repoID int, branch, location string) ([]*api.Machine, error) GetCodespaceRepositoryContents(ctx context.Context, codespace *api.Codespace, path string) ([]byte, error) } diff --git a/pkg/cmd/codespace/create.go b/pkg/cmd/codespace/create.go index 68f6d5a86..3f8aae2ca 100644 --- a/pkg/cmd/codespace/create.go +++ b/pkg/cmd/codespace/create.go @@ -71,7 +71,7 @@ func (a *App) Create(ctx context.Context, opts createOptions) error { return fmt.Errorf("error getting codespace user: %w", userResult.Err) } - machine, err := getMachineName(ctx, opts.machine, userResult.User, repository, branch, locationResult.Location, a.apiClient) + machine, err := getMachineName(ctx, a.apiClient, repository.ID, opts.machine, branch, locationResult.Location) if err != nil { return fmt.Errorf("error getting machine type: %w", err) } @@ -234,8 +234,8 @@ func getBranchName(branch string) (string, error) { } // getMachineName prompts the user to select the machine type, or validates the machine if non-empty. -func getMachineName(ctx context.Context, machine string, user *api.User, repo *api.Repository, branch, location string, apiClient apiClient) (string, error) { - skus, err := apiClient.GetCodespacesSKUs(ctx, user, repo, branch, location) +func getMachineName(ctx context.Context, apiClient apiClient, repoID int, machine, branch, location string) (string, error) { + machines, err := apiClient.GetCodespacesMachines(ctx, repoID, branch, location) if err != nil { return "", fmt.Errorf("error requesting machine instance types: %w", err) } @@ -243,55 +243,55 @@ func getMachineName(ctx context.Context, machine string, user *api.User, repo *a // if user supplied a machine type, it must be valid // if no machine type was supplied, we don't error if there are no machine types for the current repo if machine != "" { - for _, sku := range skus { - if machine == sku.Name { + for _, m := range machines { + if machine == m.Name { return machine, nil } } - availableSKUs := make([]string, len(skus)) - for i := 0; i < len(skus); i++ { - availableSKUs[i] = skus[i].Name + availableMachines := make([]string, len(machines)) + for i := 0; i < len(machines); i++ { + availableMachines[i] = machines[i].Name } - return "", fmt.Errorf("there is no such machine for the repository: %s\nAvailable machines: %v", machine, availableSKUs) - } else if len(skus) == 0 { + return "", fmt.Errorf("there is no such machine for the repository: %s\nAvailable machines: %v", machine, availableMachines) + } else if len(machines) == 0 { return "", nil } - if len(skus) == 1 { - return skus[0].Name, nil // VS Code does not prompt for SKU if there is only one, this makes us consistent with that behavior + if len(machines) == 1 { + // VS Code does not prompt for machine if there is only one, this makes us consistent with that behavior + return machines[0].Name, nil } - skuNames := make([]string, 0, len(skus)) - skuByName := make(map[string]*api.SKU) - for _, sku := range skus { - nameParts := camelcase.Split(sku.Name) + machineNames := make([]string, 0, len(machines)) + machineByName := make(map[string]*api.Machine) + for _, m := range machines { + nameParts := camelcase.Split(m.Name) machineName := strings.Title(strings.ToLower(nameParts[0])) - skuName := fmt.Sprintf("%s - %s", machineName, sku.DisplayName) - skuNames = append(skuNames, skuName) - skuByName[skuName] = sku + machineName = fmt.Sprintf("%s - %s", machineName, m.DisplayName) + machineNames = append(machineNames, machineName) + machineByName[machineName] = m } - skuSurvey := []*survey.Question{ + machineSurvey := []*survey.Question{ { - Name: "sku", + Name: "machine", Prompt: &survey.Select{ Message: "Choose Machine Type:", - Options: skuNames, - Default: skuNames[0], + Options: machineNames, + Default: machineNames[0], }, Validate: survey.Required, }, } - var skuAnswers struct{ SKU string } - if err := ask(skuSurvey, &skuAnswers); err != nil { - return "", fmt.Errorf("error getting SKU: %w", err) + var machineAnswers struct{ Machine string } + if err := ask(machineSurvey, &machineAnswers); err != nil { + return "", fmt.Errorf("error getting machine: %w", err) } - sku := skuByName[skuAnswers.SKU] - machine = sku.Name + selectedMachine := machineByName[machineAnswers.Machine] - return machine, nil + return selectedMachine.Name, nil } diff --git a/pkg/cmd/codespace/mock_api.go b/pkg/cmd/codespace/mock_api.go index bda277e34..6c845a8d5 100644 --- a/pkg/cmd/codespace/mock_api.go +++ b/pkg/cmd/codespace/mock_api.go @@ -37,8 +37,8 @@ import ( // GetCodespaceTokenFunc: func(ctx context.Context, user string, name string) (string, error) { // panic("mock out the GetCodespaceToken method") // }, -// GetCodespacesSKUsFunc: func(ctx context.Context, user *api.User, repository *api.Repository, branch string, location string) ([]*api.SKU, error) { -// panic("mock out the GetCodespacesSKUs method") +// GetCodespacesMachinesFunc: func(ctx context.Context, repoID int, branch string, location string) ([]*api.Machine, error) { +// panic("mock out the GetCodespacesMachines method") // }, // GetRepositoryFunc: func(ctx context.Context, nwo string) (*api.Repository, error) { // panic("mock out the GetRepository method") @@ -80,8 +80,8 @@ type apiClientMock struct { // GetCodespaceTokenFunc mocks the GetCodespaceToken method. GetCodespaceTokenFunc func(ctx context.Context, user string, name string) (string, error) - // GetCodespacesSKUsFunc mocks the GetCodespacesSKUs method. - GetCodespacesSKUsFunc func(ctx context.Context, user *api.User, repository *api.Repository, branch string, location string) ([]*api.SKU, error) + // GetCodespacesMachinesFunc mocks the GetCodespacesMachines method. + GetCodespacesMachinesFunc func(ctx context.Context, repoID int, branch string, location string) ([]*api.Machine, error) // GetRepositoryFunc mocks the GetRepository method. GetRepositoryFunc func(ctx context.Context, nwo string) (*api.Repository, error) @@ -152,14 +152,12 @@ type apiClientMock struct { // Name is the name argument value. Name string } - // GetCodespacesSKUs holds details about calls to the GetCodespacesSKUs method. - GetCodespacesSKUs []struct { + // GetCodespacesMachines holds details about calls to the GetCodespacesMachines method. + GetCodespacesMachines []struct { // Ctx is the ctx argument value. Ctx context.Context - // User is the user argument value. - User *api.User - // Repository is the repository argument value. - Repository *api.Repository + // RepoID is the repoID argument value. + RepoID int // Branch is the branch argument value. Branch string // Location is the location argument value. @@ -197,7 +195,7 @@ type apiClientMock struct { lockGetCodespaceRegionLocation sync.RWMutex lockGetCodespaceRepositoryContents sync.RWMutex lockGetCodespaceToken sync.RWMutex - lockGetCodespacesSKUs sync.RWMutex + lockGetCodespacesMachines sync.RWMutex lockGetRepository sync.RWMutex lockGetUser sync.RWMutex lockListCodespaces sync.RWMutex @@ -461,50 +459,46 @@ func (mock *apiClientMock) GetCodespaceTokenCalls() []struct { return calls } -// GetCodespacesSKUs calls GetCodespacesSKUsFunc. -func (mock *apiClientMock) GetCodespacesSKUs(ctx context.Context, user *api.User, repository *api.Repository, branch string, location string) ([]*api.SKU, error) { - if mock.GetCodespacesSKUsFunc == nil { - panic("apiClientMock.GetCodespacesSKUsFunc: method is nil but apiClient.GetCodespacesSKUs was just called") +// GetCodespacesMachines calls GetCodespacesMachinesFunc. +func (mock *apiClientMock) GetCodespacesMachines(ctx context.Context, repoID int, branch string, location string) ([]*api.Machine, error) { + if mock.GetCodespacesMachinesFunc == nil { + panic("apiClientMock.GetCodespacesMachinesFunc: method is nil but apiClient.GetCodespacesMachines was just called") } callInfo := struct { - Ctx context.Context - User *api.User - Repository *api.Repository - Branch string - Location string + Ctx context.Context + RepoID int + Branch string + Location string }{ - Ctx: ctx, - User: user, - Repository: repository, - Branch: branch, - Location: location, + Ctx: ctx, + RepoID: repoID, + Branch: branch, + Location: location, } - mock.lockGetCodespacesSKUs.Lock() - mock.calls.GetCodespacesSKUs = append(mock.calls.GetCodespacesSKUs, callInfo) - mock.lockGetCodespacesSKUs.Unlock() - return mock.GetCodespacesSKUsFunc(ctx, user, repository, branch, location) + mock.lockGetCodespacesMachines.Lock() + mock.calls.GetCodespacesMachines = append(mock.calls.GetCodespacesMachines, callInfo) + mock.lockGetCodespacesMachines.Unlock() + return mock.GetCodespacesMachinesFunc(ctx, repoID, branch, location) } -// GetCodespacesSKUsCalls gets all the calls that were made to GetCodespacesSKUs. +// GetCodespacesMachinesCalls gets all the calls that were made to GetCodespacesMachines. // Check the length with: -// len(mockedapiClient.GetCodespacesSKUsCalls()) -func (mock *apiClientMock) GetCodespacesSKUsCalls() []struct { - Ctx context.Context - User *api.User - Repository *api.Repository - Branch string - Location string +// len(mockedapiClient.GetCodespacesMachinesCalls()) +func (mock *apiClientMock) GetCodespacesMachinesCalls() []struct { + Ctx context.Context + RepoID int + Branch string + Location string } { var calls []struct { - Ctx context.Context - User *api.User - Repository *api.Repository - Branch string - Location string + Ctx context.Context + RepoID int + Branch string + Location string } - mock.lockGetCodespacesSKUs.RLock() - calls = mock.calls.GetCodespacesSKUs - mock.lockGetCodespacesSKUs.RUnlock() + mock.lockGetCodespacesMachines.RLock() + calls = mock.calls.GetCodespacesMachines + mock.lockGetCodespacesMachines.RUnlock() return calls } From d02876e6ea3cc0ad12437cd5ecd684e9d0d226ca Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Mon, 4 Oct 2021 14:16:04 -0400 Subject: [PATCH 5/5] Rename the cmd pkg to codespace --- pkg/cmd/codespace/code.go | 2 +- pkg/cmd/codespace/common.go | 4 ++-- pkg/cmd/codespace/create.go | 2 +- pkg/cmd/codespace/delete.go | 2 +- pkg/cmd/codespace/delete_test.go | 2 +- pkg/cmd/codespace/list.go | 2 +- pkg/cmd/codespace/logs.go | 2 +- pkg/cmd/codespace/mock_api.go | 2 +- pkg/cmd/codespace/mock_prompter.go | 2 +- pkg/cmd/codespace/ports.go | 4 ++-- pkg/cmd/codespace/root.go | 4 ++-- pkg/cmd/codespace/ssh.go | 2 +- 12 files changed, 15 insertions(+), 15 deletions(-) diff --git a/pkg/cmd/codespace/code.go b/pkg/cmd/codespace/code.go index 0448d8b9f..4261bcf7d 100644 --- a/pkg/cmd/codespace/code.go +++ b/pkg/cmd/codespace/code.go @@ -1,4 +1,4 @@ -package ghcs +package codespace import ( "context" diff --git a/pkg/cmd/codespace/common.go b/pkg/cmd/codespace/common.go index ecce47b2a..58ab4f738 100644 --- a/pkg/cmd/codespace/common.go +++ b/pkg/cmd/codespace/common.go @@ -1,6 +1,6 @@ -package ghcs +package codespace -// This file defines functions common to the entire ghcs command set. +// This file defines functions common to the entire codespace command set. import ( "context" diff --git a/pkg/cmd/codespace/create.go b/pkg/cmd/codespace/create.go index 68f6d5a86..2f0f628f5 100644 --- a/pkg/cmd/codespace/create.go +++ b/pkg/cmd/codespace/create.go @@ -1,4 +1,4 @@ -package ghcs +package codespace import ( "context" diff --git a/pkg/cmd/codespace/delete.go b/pkg/cmd/codespace/delete.go index 8ea821dc9..ac483350c 100644 --- a/pkg/cmd/codespace/delete.go +++ b/pkg/cmd/codespace/delete.go @@ -1,4 +1,4 @@ -package ghcs +package codespace import ( "context" diff --git a/pkg/cmd/codespace/delete_test.go b/pkg/cmd/codespace/delete_test.go index bf16f82f0..fcb63c4db 100644 --- a/pkg/cmd/codespace/delete_test.go +++ b/pkg/cmd/codespace/delete_test.go @@ -1,4 +1,4 @@ -package ghcs +package codespace import ( "bytes" diff --git a/pkg/cmd/codespace/list.go b/pkg/cmd/codespace/list.go index a6a4b9a1b..9164b7921 100644 --- a/pkg/cmd/codespace/list.go +++ b/pkg/cmd/codespace/list.go @@ -1,4 +1,4 @@ -package ghcs +package codespace import ( "context" diff --git a/pkg/cmd/codespace/logs.go b/pkg/cmd/codespace/logs.go index 0e5a7704d..0e52b488a 100644 --- a/pkg/cmd/codespace/logs.go +++ b/pkg/cmd/codespace/logs.go @@ -1,4 +1,4 @@ -package ghcs +package codespace import ( "context" diff --git a/pkg/cmd/codespace/mock_api.go b/pkg/cmd/codespace/mock_api.go index 669083a32..1cbf4d1db 100644 --- a/pkg/cmd/codespace/mock_api.go +++ b/pkg/cmd/codespace/mock_api.go @@ -1,7 +1,7 @@ // Code generated by moq; DO NOT EDIT. // github.com/matryer/moq -package ghcs +package codespace import ( "context" diff --git a/pkg/cmd/codespace/mock_prompter.go b/pkg/cmd/codespace/mock_prompter.go index 56581b64d..3ce257a39 100644 --- a/pkg/cmd/codespace/mock_prompter.go +++ b/pkg/cmd/codespace/mock_prompter.go @@ -1,7 +1,7 @@ // Code generated by moq; DO NOT EDIT. // github.com/matryer/moq -package ghcs +package codespace import ( "sync" diff --git a/pkg/cmd/codespace/ports.go b/pkg/cmd/codespace/ports.go index dee726bc4..d2c14a388 100644 --- a/pkg/cmd/codespace/ports.go +++ b/pkg/cmd/codespace/ports.go @@ -1,4 +1,4 @@ -package ghcs +package codespace import ( "bytes" @@ -79,7 +79,7 @@ func (a *App) ListPorts(ctx context.Context, codespaceName string, asJSON bool) devContainerResult := <-devContainerCh if devContainerResult.err != nil { - // Warn about failure to read the devcontainer file. Not a ghcs command error. + // Warn about failure to read the devcontainer file. Not a codespace command error. _, _ = a.logger.Errorf("Failed to get port names: %v\n", devContainerResult.err.Error()) } diff --git a/pkg/cmd/codespace/root.go b/pkg/cmd/codespace/root.go index c9fdd2876..bd566bb5a 100644 --- a/pkg/cmd/codespace/root.go +++ b/pkg/cmd/codespace/root.go @@ -1,4 +1,4 @@ -package ghcs +package codespace import ( "github.com/spf13/cobra" @@ -8,7 +8,7 @@ var version = "DEV" // Replaced in the release build process (by GoReleaser or H func NewRootCmd(app *App) *cobra.Command { root := &cobra.Command{ - Use: "ghcs", + Use: "codespace", SilenceUsage: true, // don't print usage message after each error (see #80) SilenceErrors: false, // print errors automatically so that main need not Long: `Unofficial CLI tool to manage GitHub Codespaces. diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 1a3d48335..a7bb682ba 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -1,4 +1,4 @@ -package ghcs +package codespace import ( "context"