Simplify delete further

This commit is contained in:
Mislav Marohnić 2021-09-21 21:09:26 +02:00
parent ab86739b6b
commit 678da44c28
6 changed files with 58 additions and 122 deletions

View file

@ -19,7 +19,7 @@ import (
var errNoCodespaces = errors.New("you have no codespaces")
func chooseCodespace(ctx context.Context, apiClient *api.API, user *api.User) (*api.Codespace, error) {
codespaces, err := apiClient.ListCodespaces(ctx, user)
codespaces, err := apiClient.ListCodespaces(ctx, user.Login)
if err != nil {
return nil, fmt.Errorf("error getting codespaces: %w", err)
}

View file

@ -2,13 +2,13 @@ package ghcs
import (
"context"
"errors"
"fmt"
"os"
"strings"
"time"
"github.com/AlecAivazis/survey/v2"
"github.com/github/ghcs/cmd/ghcs/output"
"github.com/github/ghcs/internal/api"
"github.com/spf13/cobra"
"golang.org/x/sync/errgroup"
@ -17,19 +17,32 @@ import (
type deleteOptions struct {
deleteAll bool
skipConfirm bool
isInteractive bool
codespaceName string
repoFilter string
keepDays uint16
isInteractive bool
now func() time.Time
apiClient *api.API
apiClient apiClient
prompter prompter
}
type prompter interface {
Confirm(message string) (bool, error)
}
type apiClient interface {
GetUser(ctx context.Context) (*api.User, error)
ListCodespaces(ctx context.Context, user string) ([]*api.Codespace, error)
DeleteCodespace(ctx context.Context, user, name string) error
}
func newDeleteCmd() *cobra.Command {
opts := deleteOptions{
apiClient: api.New(os.Getenv("GITHUB_TOKEN")),
now: time.Now,
isInteractive: hasTTY,
now: time.Now,
apiClient: api.New(os.Getenv("GITHUB_TOKEN")),
prompter: &surveyPrompter{},
}
deleteCmd := &cobra.Command{
@ -37,15 +50,10 @@ func newDeleteCmd() *cobra.Command {
Short: "Delete a codespace",
Args: cobra.NoArgs,
RunE: func(cmd *cobra.Command, args []string) error {
// switch {
// case allCodespaces && repo != "":
// return errors.New("both --all and --repo is not supported")
// case allCodespaces:
// return deleteAll(log, force, keepThresholdDays)
// case repo != "":
// return deleteByRepo(log, repo, force, keepThresholdDays)
log := output.NewLogger(os.Stdout, os.Stderr, false)
return delete(context.Background(), log, opts)
if opts.deleteAll && opts.repoFilter != "" {
return errors.New("both --all and --repo is not supported")
}
return delete(context.Background(), opts)
},
}
@ -58,13 +66,13 @@ func newDeleteCmd() *cobra.Command {
return deleteCmd
}
func delete(ctx context.Context, log *output.Logger, opts deleteOptions) error {
func delete(ctx context.Context, opts deleteOptions) error {
user, err := opts.apiClient.GetUser(ctx)
if err != nil {
return fmt.Errorf("error getting user: %w", err)
}
codespaces, err := opts.apiClient.ListCodespaces(ctx, user)
codespaces, err := opts.apiClient.ListCodespaces(ctx, user.Login)
if err != nil {
return fmt.Errorf("error getting codespaces: %w", err)
}
@ -78,7 +86,7 @@ func delete(ctx context.Context, log *output.Logger, opts deleteOptions) error {
nameFilter = c.Name
}
var codespacesToDelete []*api.Codespace
codespacesToDelete := make([]*api.Codespace, 0, len(codespaces))
lastUpdatedCutoffTime := opts.now().AddDate(0, 0, -int(opts.keepDays))
for _, c := range codespaces {
if nameFilter != "" && c.Name != nameFilter {
@ -97,9 +105,9 @@ func delete(ctx context.Context, log *output.Logger, opts deleteOptions) error {
}
}
if nameFilter == "" || !opts.skipConfirm {
confirmed, err := confirmDeletion(c)
confirmed, err := confirmDeletion(opts.prompter, c, opts.isInteractive)
if err != nil {
return fmt.Errorf("deletion could not be confirmed: %w", err)
return fmt.Errorf("unable to confirm: %w", err)
}
if !confirmed {
continue
@ -112,11 +120,7 @@ func delete(ctx context.Context, log *output.Logger, opts deleteOptions) error {
for _, c := range codespacesToDelete {
codespaceName := c.Name
g.Go(func() error {
token, err := opts.apiClient.GetCodespaceToken(ctx, user.Login, codespaceName)
if err != nil {
return fmt.Errorf("error getting codespace token: %w", err)
}
if err := opts.apiClient.DeleteCodespace(ctx, user, token, codespaceName); err != nil {
if err := opts.apiClient.DeleteCodespace(ctx, user.Login, codespaceName); err != nil {
return fmt.Errorf("error deleting codespace: %w", err)
}
return nil
@ -126,16 +130,21 @@ func delete(ctx context.Context, log *output.Logger, opts deleteOptions) error {
return g.Wait()
}
func confirmDeletion(codespace *api.Codespace) (bool, error) {
func confirmDeletion(p prompter, codespace *api.Codespace, isInteractive bool) (bool, error) {
gs := codespace.Environment.GitStatus
hasUnsavedChanges := gs.HasUncommitedChanges || gs.HasUnpushedChanges
if !hasUnsavedChanges {
return true, nil
}
if !hasTTY {
if !isInteractive {
return false, fmt.Errorf("codespace %s has unsaved changes (use --force to override)", codespace.Name)
}
return p.Confirm(fmt.Sprintf("Codespace %s has unsaved changes. OK to delete?", codespace.Name))
}
type surveyPrompter struct{}
func (p *surveyPrompter) Confirm(message string) (bool, error) {
var confirmed struct {
Confirmed bool
}
@ -143,7 +152,7 @@ func confirmDeletion(codespace *api.Codespace) (bool, error) {
{
Name: "confirmed",
Prompt: &survey.Confirm{
Message: fmt.Sprintf("Codespace %s has unsaved changes. OK to delete?", codespace.Name),
Message: message,
},
},
}

View file

@ -1,103 +1,28 @@
package ghcs
import (
"context"
"testing"
"time"
"github.com/github/ghcs/internal/api"
)
func TestFilterCodespacesToDelete(t *testing.T) {
type args struct {
codespaces []*api.Codespace
thresholdDays int
}
func TestDelete(t *testing.T) {
tests := []struct {
name string
now time.Time
args args
opts deleteOptions
wantErr bool
deleted []*api.Codespace
}{
{
name: "no codespaces is to be deleted",
args: args{
codespaces: []*api.Codespace{
{
Name: "testcodespace",
CreatedAt: "2021-08-09T10:10:24+02:00",
LastUsedAt: "2021-08-09T13:10:24+02:00",
Environment: api.CodespaceEnvironment{
State: "Shutdown",
},
},
},
thresholdDays: 1,
name: "by name",
opts: deleteOptions{
codespaceName: "foo-bar-123",
},
now: time.Date(2021, 8, 9, 20, 10, 24, 0, time.UTC),
deleted: []*api.Codespace{},
},
{
name: "one codespace is to be deleted",
args: args{
codespaces: []*api.Codespace{
{
Name: "testcodespace",
CreatedAt: "2021-08-09T10:10:24+02:00",
LastUsedAt: "2021-08-09T13:10:24+02:00",
Environment: api.CodespaceEnvironment{
State: "Shutdown",
},
},
},
thresholdDays: 1,
},
now: time.Date(2021, 8, 15, 20, 12, 24, 0, time.UTC),
deleted: []*api.Codespace{
{
Name: "testcodespace",
CreatedAt: "2021-08-09T10:10:24+02:00",
LastUsedAt: "2021-08-09T13:10:24+02:00",
},
},
},
{
name: "threshold is invalid",
args: args{
codespaces: []*api.Codespace{
{
Name: "testcodespace",
CreatedAt: "2021-08-09T10:10:24+02:00",
LastUsedAt: "2021-08-09T13:10:24+02:00",
Environment: api.CodespaceEnvironment{
State: "Shutdown",
},
},
},
thresholdDays: -1,
},
now: time.Date(2021, 8, 15, 20, 12, 24, 0, time.UTC),
wantErr: true,
deleted: []*api.Codespace{},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
now = func() time.Time {
return tt.now
}
codespaces, err := filterCodespacesToDelete(tt.args.codespaces, tt.args.thresholdDays)
err := delete(context.Background(), tt.opts)
if (err != nil) != tt.wantErr {
t.Errorf("API.CleanupUnusedCodespaces() error = %v, wantErr %v", err, tt.wantErr)
}
if len(codespaces) != len(tt.deleted) {
t.Errorf("expected %d deleted codespaces, got %d", len(tt.deleted), len(codespaces))
t.Errorf("delete() error = %v, wantErr %v", err, tt.wantErr)
}
})
}

View file

@ -40,7 +40,7 @@ func list(opts *listOptions) error {
return fmt.Errorf("error getting user: %w", err)
}
codespaces, err := apiClient.ListCodespaces(ctx, user)
codespaces, err := apiClient.ListCodespaces(ctx, user.Login)
if err != nil {
return fmt.Errorf("error getting codespaces: %w", err)
}

View file

@ -32,11 +32,12 @@ import (
"encoding/json"
"errors"
"fmt"
"github.com/opentracing/opentracing-go"
"io/ioutil"
"net/http"
"strconv"
"strings"
"github.com/opentracing/opentracing-go"
)
const githubAPI = "https://api.github.com"
@ -172,9 +173,9 @@ type CodespaceEnvironmentConnection struct {
RelaySAS string `json:"relaySas"`
}
func (a *API) ListCodespaces(ctx context.Context, user *User) ([]*Codespace, error) {
func (a *API) ListCodespaces(ctx context.Context, user string) ([]*Codespace, error) {
req, err := http.NewRequest(
http.MethodGet, a.githubAPI+"/vscs_internal/user/"+user.Login+"/codespaces", nil,
http.MethodGet, a.githubAPI+"/vscs_internal/user/"+user+"/codespaces", nil,
)
if err != nil {
return nil, fmt.Errorf("error creating request: %w", err)
@ -442,8 +443,13 @@ func (a *API) CreateCodespace(ctx context.Context, user *User, repository *Repos
return &response, nil
}
func (a *API) DeleteCodespace(ctx context.Context, user *User, token, codespaceName string) error {
req, err := http.NewRequest(http.MethodDelete, a.githubAPI+"/vscs_internal/user/"+user.Login+"/codespaces/"+codespaceName, 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)
if err != nil {
return fmt.Errorf("error creating request: %w", err)
}

View file

@ -10,10 +10,6 @@ import (
)
func TestListCodespaces(t *testing.T) {
user := &User{
Login: "testuser",
}
codespaces := []*Codespace{
{
Name: "testcodespace",
@ -38,7 +34,7 @@ func TestListCodespaces(t *testing.T) {
token: "faketoken",
}
ctx := context.TODO()
codespaces, err := api.ListCodespaces(ctx, user)
codespaces, err := api.ListCodespaces(ctx, "testuser")
if err != nil {
t.Fatal(err)
}