From 41719f762414e882cf8f8d1a0d06a3b9e38f3b85 Mon Sep 17 00:00:00 2001 From: Jake Shorty Date: Tue, 5 Apr 2022 20:19:19 +0000 Subject: [PATCH 1/4] Location is an optional flag when creating codespaces --- internal/codespaces/api/api.go | 34 ---------------------------- pkg/cmd/codespace/common.go | 1 - pkg/cmd/codespace/create.go | 38 +++++++++----------------------- pkg/cmd/codespace/create_test.go | 6 ----- 4 files changed, 10 insertions(+), 69 deletions(-) diff --git a/internal/codespaces/api/api.go b/internal/codespaces/api/api.go index 49b41b7e5..8e3008d8d 100644 --- a/internal/codespaces/api/api.go +++ b/internal/codespaces/api/api.go @@ -432,40 +432,6 @@ func (a *API) StopCodespace(ctx context.Context, codespaceName string) error { return nil } -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, a.vscsAPI+"/api/v1/locations", nil) - if err != nil { - return "", fmt.Errorf("error creating request: %w", err) - } - - resp, err := a.do(ctx, req, req.URL.String()) - if err != nil { - return "", fmt.Errorf("error making request: %w", err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return "", api.HandleHTTPError(resp) - } - - b, err := ioutil.ReadAll(resp.Body) - if err != nil { - return "", fmt.Errorf("error reading response body: %w", err) - } - - var response getCodespaceRegionLocationResponse - if err := json.Unmarshal(b, &response); err != nil { - return "", fmt.Errorf("error unmarshaling response: %w", err) - } - - return response.Current, nil -} - type Machine struct { Name string `json:"name"` DisplayName string `json:"display_name"` diff --git a/pkg/cmd/codespace/common.go b/pkg/cmd/codespace/common.go index aec386f6c..dde1a0f30 100644 --- a/pkg/cmd/codespace/common.go +++ b/pkg/cmd/codespace/common.go @@ -71,7 +71,6 @@ type apiClient interface { EditCodespace(ctx context.Context, codespaceName string, params *api.EditCodespaceParams) (*api.Codespace, error) GetRepository(ctx context.Context, nwo string) (*api.Repository, error) AuthorizedKeys(ctx context.Context, user string) ([]byte, error) - GetCodespaceRegionLocation(ctx context.Context) (string, error) GetCodespacesMachines(ctx context.Context, repoID int, branch, location string) ([]*api.Machine, error) GetCodespaceRepositoryContents(ctx context.Context, codespace *api.Codespace, path string) ([]byte, error) GetCodespaceRepoSuggestions(ctx context.Context, partialSearch string, params api.RepoSearchParameters) ([]string, error) diff --git a/pkg/cmd/codespace/create.go b/pkg/cmd/codespace/create.go index 81bb57bc4..5da87f240 100644 --- a/pkg/cmd/codespace/create.go +++ b/pkg/cmd/codespace/create.go @@ -18,6 +18,7 @@ import ( type createOptions struct { repo string branch string + location string machine string showStatus bool permissionsOptOut bool @@ -38,6 +39,7 @@ func newCreateCmd(app *App) *cobra.Command { createCmd.Flags().StringVarP(&opts.repo, "repo", "r", "", "repository name with owner: user/repo") createCmd.Flags().StringVarP(&opts.branch, "branch", "b", "", "repository branch") + createCmd.Flags().StringVarP(&opts.location, "location", "l", "", "location (assigned by IP or user settings if not provided)") createCmd.Flags().StringVarP(&opts.machine, "machine", "m", "", "hardware specifications for the VM") createCmd.Flags().BoolVarP(&opts.permissionsOptOut, "default-permissions", "", false, "do not prompt to accept additional permissions requested by the codespace") createCmd.Flags().BoolVarP(&opts.showStatus, "status", "s", false, "show status of post-create command and dotfiles") @@ -54,14 +56,14 @@ func (a *App) Create(ctx context.Context, opts createOptions) error { vscsTarget := os.Getenv("VSCS_TARGET") vscsTargetUrl := os.Getenv("VSCS_TARGET_URL") - locationCh := getLocation(ctx, vscsLocation, a.apiClient) - userInputs := struct { Repository string Branch string + Location string }{ Repository: opts.repo, Branch: opts.branch, + Location: opts.location, } if userInputs.Repository == "" { @@ -94,6 +96,10 @@ func (a *App) Create(ctx context.Context, opts createOptions) error { } } + if userInputs.Location == "" && vscsLocation != "" { + userInputs.Location = vscsLocation + } + a.StartProgressIndicatorWithLabel("Fetching repository") repository, err := a.apiClient.GetRepository(ctx, userInputs.Repository) a.StopProgressIndicator() @@ -106,12 +112,7 @@ func (a *App) Create(ctx context.Context, opts createOptions) error { branch = repository.DefaultBranch } - locationResult := <-locationCh - if locationResult.Err != nil { - return fmt.Errorf("error getting codespace region location: %w", locationResult.Err) - } - - machine, err := getMachineName(ctx, a.apiClient, repository.ID, opts.machine, branch, locationResult.Location) + machine, err := getMachineName(ctx, a.apiClient, repository.ID, opts.machine, branch, userInputs.Location) if err != nil { return fmt.Errorf("error getting machine type: %w", err) } @@ -123,7 +124,7 @@ func (a *App) Create(ctx context.Context, opts createOptions) error { RepositoryID: repository.ID, Branch: branch, Machine: machine, - Location: locationResult.Location, + Location: userInputs.Location, VSCSTarget: vscsTarget, VSCSTargetURL: vscsTargetUrl, IdleTimeoutMinutes: int(opts.idleTimeout.Minutes()), @@ -292,25 +293,6 @@ type locationResult struct { Err error } -// getLocation fetches the closest Codespace datacenter -// region/location to the user, unless the 'vscsLocationOverride' override is set -func getLocation(ctx context.Context, vscsLocationOverride string, apiClient apiClient) <-chan locationResult { - ch := make(chan locationResult, 1) - - // Developer override is set, return the override - if vscsLocationOverride != "" { - ch <- locationResult{vscsLocationOverride, nil} - return ch - } - - // Dynamically fetch the region location - go func() { - location, err := apiClient.GetCodespaceRegionLocation(ctx) - ch <- locationResult{location, err} - }() - return ch -} - // getMachineName prompts the user to select the machine type, or validates the machine if non-empty. func getMachineName(ctx context.Context, apiClient apiClient, repoID int, machine, branch, location string) (string, error) { machines, err := apiClient.GetCodespacesMachines(ctx, repoID, branch, location) diff --git a/pkg/cmd/codespace/create_test.go b/pkg/cmd/codespace/create_test.go index 2d417620d..76f4af88c 100644 --- a/pkg/cmd/codespace/create_test.go +++ b/pkg/cmd/codespace/create_test.go @@ -27,9 +27,6 @@ func TestApp_Create(t *testing.T) { name: "create codespace with default branch and 30m idle timeout", fields: fields{ apiClient: &apiClientMock{ - GetCodespaceRegionLocationFunc: func(ctx context.Context) (string, error) { - return "EUROPE", nil - }, GetRepositoryFunc: func(ctx context.Context, nwo string) (*api.Repository, error) { return &api.Repository{ ID: 1234, @@ -74,9 +71,6 @@ func TestApp_Create(t *testing.T) { name: "create codespace that requires accepting additional permissions", fields: fields{ apiClient: &apiClientMock{ - GetCodespaceRegionLocationFunc: func(ctx context.Context) (string, error) { - return "EUROPE", nil - }, GetRepositoryFunc: func(ctx context.Context, nwo string) (*api.Repository, error) { return &api.Repository{ ID: 1234, From 4aca945e2e05d15bcc772b7ab837e7dcb595b5a0 Mon Sep 17 00:00:00 2001 From: Jake Shorty Date: Tue, 5 Apr 2022 20:27:10 +0000 Subject: [PATCH 2/4] Drop unused locationResult --- pkg/cmd/codespace/create.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pkg/cmd/codespace/create.go b/pkg/cmd/codespace/create.go index 5da87f240..b0f0b6b9c 100644 --- a/pkg/cmd/codespace/create.go +++ b/pkg/cmd/codespace/create.go @@ -288,11 +288,6 @@ func (a *App) showStatus(ctx context.Context, codespace *api.Codespace) error { return nil } -type locationResult struct { - Location string - Err error -} - // getMachineName prompts the user to select the machine type, or validates the machine if non-empty. func getMachineName(ctx context.Context, apiClient apiClient, repoID int, machine, branch, location string) (string, error) { machines, err := apiClient.GetCodespacesMachines(ctx, repoID, branch, location) From be7a4ed70b3564a2c7a6ac85b1d412ac359aa96a Mon Sep 17 00:00:00 2001 From: Jake Shorty Date: Wed, 6 Apr 2022 14:57:59 +0000 Subject: [PATCH 3/4] Enumerate available codespace locations for now We'll probably want to have an intermediate geographical abstraction for these eventually, but this gives parity with the github.com UI. --- pkg/cmd/codespace/create.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/codespace/create.go b/pkg/cmd/codespace/create.go index b0f0b6b9c..298e5ffee 100644 --- a/pkg/cmd/codespace/create.go +++ b/pkg/cmd/codespace/create.go @@ -39,7 +39,7 @@ func newCreateCmd(app *App) *cobra.Command { createCmd.Flags().StringVarP(&opts.repo, "repo", "r", "", "repository name with owner: user/repo") createCmd.Flags().StringVarP(&opts.branch, "branch", "b", "", "repository branch") - createCmd.Flags().StringVarP(&opts.location, "location", "l", "", "location (assigned by IP or user settings if not provided)") + createCmd.Flags().StringVarP(&opts.location, "location", "l", "", "location (EastUs, SouthEastAsia, WestEurope, or WestUs2). Assigned by IP or user settings if not provided") createCmd.Flags().StringVarP(&opts.machine, "machine", "m", "", "hardware specifications for the VM") createCmd.Flags().BoolVarP(&opts.permissionsOptOut, "default-permissions", "", false, "do not prompt to accept additional permissions requested by the codespace") createCmd.Flags().BoolVarP(&opts.showStatus, "status", "s", false, "show status of post-create command and dotfiles") From e9ed200c1bd78a8af8ff532c3d833da27faffd96 Mon Sep 17 00:00:00 2001 From: Jake Shorty Date: Thu, 7 Apr 2022 10:49:03 -0600 Subject: [PATCH 4/4] Use proper syntax for location values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Mislav Marohnić --- pkg/cmd/codespace/create.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/codespace/create.go b/pkg/cmd/codespace/create.go index 298e5ffee..74a1fe830 100644 --- a/pkg/cmd/codespace/create.go +++ b/pkg/cmd/codespace/create.go @@ -39,7 +39,7 @@ func newCreateCmd(app *App) *cobra.Command { createCmd.Flags().StringVarP(&opts.repo, "repo", "r", "", "repository name with owner: user/repo") createCmd.Flags().StringVarP(&opts.branch, "branch", "b", "", "repository branch") - createCmd.Flags().StringVarP(&opts.location, "location", "l", "", "location (EastUs, SouthEastAsia, WestEurope, or WestUs2). Assigned by IP or user settings if not provided") + createCmd.Flags().StringVarP(&opts.location, "location", "l", "", "location: {EastUs|SouthEastAsia|WestEurope|WestUs2} (determined automatically if not provided)") createCmd.Flags().StringVarP(&opts.machine, "machine", "m", "", "hardware specifications for the VM") createCmd.Flags().BoolVarP(&opts.permissionsOptOut, "default-permissions", "", false, "do not prompt to accept additional permissions requested by the codespace") createCmd.Flags().BoolVarP(&opts.showStatus, "status", "s", false, "show status of post-create command and dotfiles")