From 6b1876161d696629bdf8e2e0d8c34951313fb8c9 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Fri, 1 Oct 2021 12:53:35 -0400 Subject: [PATCH 1/2] 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/2] 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) }