From 7af33d090a31fc85cc62d01c339fa330ce6efb01 Mon Sep 17 00:00:00 2001 From: Mateus Marquezini Date: Mon, 20 Nov 2023 18:45:12 -0300 Subject: [PATCH 1/4] added a new error handling when the display name flag exceeds 48 characters #8356 --- pkg/cmd/codespace/create.go | 6 +++++- pkg/cmd/codespace/create_test.go | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/codespace/create.go b/pkg/cmd/codespace/create.go index fa484173d..d8b24d016 100644 --- a/pkg/cmd/codespace/create.go +++ b/pkg/cmd/codespace/create.go @@ -110,7 +110,7 @@ func newCreateCmd(app *App) *cobra.Command { createCmd.Flags().DurationVar(&opts.idleTimeout, "idle-timeout", 0, "allowed inactivity before codespace is stopped, e.g. \"10m\", \"1h\"") createCmd.Flags().Var(&opts.retentionPeriod, "retention-period", "allowed time after shutting down before the codespace is automatically deleted (maximum 30 days), e.g. \"1h\", \"72h\"") createCmd.Flags().StringVar(&opts.devContainerPath, "devcontainer-path", "", "path to the devcontainer.json file to use when creating codespace") - createCmd.Flags().StringVarP(&opts.displayName, "display-name", "d", "", "display name for the codespace") + createCmd.Flags().StringVarP(&opts.displayName, "display-name", "d", "", "display name for the codespace (48 characters or less)") return createCmd } @@ -282,6 +282,10 @@ func (a *App) Create(ctx context.Context, opts createOptions) error { } } + if len(opts.displayName) > 48 { // 48 is the max length of the display name in the API + return fmt.Errorf("error creating codespace: display name should contains 48 characters max or less") + } + createParams := &api.CreateCodespaceParams{ RepositoryID: repository.ID, Branch: branch, diff --git a/pkg/cmd/codespace/create_test.go b/pkg/cmd/codespace/create_test.go index ae85fc34a..bbb8d84d0 100644 --- a/pkg/cmd/codespace/create_test.go +++ b/pkg/cmd/codespace/create_test.go @@ -184,6 +184,25 @@ func TestApp_Create(t *testing.T) { wantStderr: " ✓ Codespaces usage for this repository is paid for by monalisa\n", wantErr: fmt.Errorf("error getting machine type: there is no such machine for the repository: %s\nAvailable machines: %v", "MEGA", []string{"GIGA", "TERA"}), }, + { + name: "create codespace with display name more than 48 characters results in error", + fields: fields{ + apiClient: apiCreateDefaults(&apiClientMock{ + CreateCodespaceFunc: func(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) { + return &api.Codespace{ + Name: "monalisa-dotfiles-abcd1234", + }, nil + }, + }), + }, + opts: createOptions{ + repo: "monalisa/dotfiles", + machine: "GIGA", + displayName: "this-is-very-long-display-name-with-49-characters", + }, + wantStderr: " ✓ Codespaces usage for this repository is paid for by monalisa\n", + wantErr: fmt.Errorf("error creating codespace: display name should contains 48 characters max or less"), + }, { name: "create codespace with devcontainer path results in selecting the correct machine type", fields: fields{ From 7a32b8b5f30c0e14ed8b7babc147e6fb172283d5 Mon Sep 17 00:00:00 2001 From: Mateus Marquezini Date: Mon, 27 Nov 2023 17:17:24 -0300 Subject: [PATCH 2/4] some code improvements after code review --- pkg/cmd/codespace/create.go | 10 +++++++--- pkg/cmd/codespace/create_test.go | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/codespace/create.go b/pkg/cmd/codespace/create.go index d8b24d016..821faa10b 100644 --- a/pkg/cmd/codespace/create.go +++ b/pkg/cmd/codespace/create.go @@ -24,6 +24,10 @@ const ( permissionsPollingTimeout = 1 * time.Minute ) +const ( + displayNameMaxLength = 48 // 48 is the max length of the display name in the API +) + var ( DEFAULT_DEVCONTAINER_DEFINITIONS = []string{".devcontainer.json", ".devcontainer/devcontainer.json"} ) @@ -110,7 +114,7 @@ func newCreateCmd(app *App) *cobra.Command { createCmd.Flags().DurationVar(&opts.idleTimeout, "idle-timeout", 0, "allowed inactivity before codespace is stopped, e.g. \"10m\", \"1h\"") createCmd.Flags().Var(&opts.retentionPeriod, "retention-period", "allowed time after shutting down before the codespace is automatically deleted (maximum 30 days), e.g. \"1h\", \"72h\"") createCmd.Flags().StringVar(&opts.devContainerPath, "devcontainer-path", "", "path to the devcontainer.json file to use when creating codespace") - createCmd.Flags().StringVarP(&opts.displayName, "display-name", "d", "", "display name for the codespace (48 characters or less)") + createCmd.Flags().StringVarP(&opts.displayName, "display-name", "d", "", fmt.Sprintf("display name for the codespace (%d characters or less)", displayNameMaxLength)) return createCmd } @@ -282,8 +286,8 @@ func (a *App) Create(ctx context.Context, opts createOptions) error { } } - if len(opts.displayName) > 48 { // 48 is the max length of the display name in the API - return fmt.Errorf("error creating codespace: display name should contains 48 characters max or less") + if len(opts.displayName) > displayNameMaxLength { + return fmt.Errorf("error creating codespace: display name should contain a maximum of %d characters", displayNameMaxLength) } createParams := &api.CreateCodespaceParams{ diff --git a/pkg/cmd/codespace/create_test.go b/pkg/cmd/codespace/create_test.go index bbb8d84d0..78039db0d 100644 --- a/pkg/cmd/codespace/create_test.go +++ b/pkg/cmd/codespace/create_test.go @@ -201,7 +201,7 @@ func TestApp_Create(t *testing.T) { displayName: "this-is-very-long-display-name-with-49-characters", }, wantStderr: " ✓ Codespaces usage for this repository is paid for by monalisa\n", - wantErr: fmt.Errorf("error creating codespace: display name should contains 48 characters max or less"), + wantErr: fmt.Errorf("error creating codespace: display name should contain a maximum of %d characters", displayNameMaxLength), }, { name: "create codespace with devcontainer path results in selecting the correct machine type", From 2f31607096b39b4954fb5a211166b2b9e627fcb6 Mon Sep 17 00:00:00 2001 From: Mika <77445020+tal66@users.noreply.github.com> Date: Wed, 29 Nov 2023 08:54:55 +0000 Subject: [PATCH 3/4] Add timeout error in `gh auth status` (#8337) --- pkg/cmd/auth/status/status.go | 20 +++++++++++++------- pkg/cmd/auth/status/status_test.go | 21 +++++++++++++++++++++ 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index a064aabc2..831dfc503 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -3,6 +3,7 @@ package status import ( "errors" "fmt" + "net" "net/http" "path/filepath" "strings" @@ -107,13 +108,18 @@ func statusRun(opts *StatusOptions) error { scopesHeader, err := shared.GetScopes(httpClient, hostname, token) if err != nil { - addMsg("%s %s: authentication failed", cs.Red("X"), hostname) - addMsg("- The %s token in %s is invalid.", cs.Bold(hostname), tokenSource) - if tokenIsWriteable { - addMsg("- To re-authenticate, run: %s %s", - cs.Bold("gh auth login -h"), cs.Bold(hostname)) - addMsg("- To forget about this host, run: %s %s", - cs.Bold("gh auth logout -h"), cs.Bold(hostname)) + var networkError net.Error + if errors.As(err, &networkError) && networkError.Timeout() { + addMsg("%s %s: timeout trying to connect to host", cs.Red("X"), hostname) + } else { + addMsg("%s %s: authentication failed", cs.Red("X"), hostname) + addMsg("- The %s token in %s is invalid.", cs.Bold(hostname), tokenSource) + if tokenIsWriteable { + addMsg("- To re-authenticate, run: %s %s", + cs.Bold("gh auth login -h"), cs.Bold(hostname)) + addMsg("- To forget about this host, run: %s %s", + cs.Bold("gh auth logout -h"), cs.Bold(hostname)) + } } failed = true continue diff --git a/pkg/cmd/auth/status/status_test.go b/pkg/cmd/auth/status/status_test.go index 07cbe0be8..dfebddda3 100644 --- a/pkg/cmd/auth/status/status_test.go +++ b/pkg/cmd/auth/status/status_test.go @@ -2,6 +2,7 @@ package status import ( "bytes" + "context" "net/http" "path/filepath" "strings" @@ -84,6 +85,26 @@ func Test_statusRun(t *testing.T) { wantOut string wantErrOut string }{ + { + name: "timeout error", + opts: &StatusOptions{ + Hostname: "joel.miller", + }, + cfgStubs: func(c *config.ConfigMock) { + c.Set("joel.miller", "oauth_token", "abc123") + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.REST("GET", "api/v3/"), func(req *http.Request) (*http.Response, error) { + // timeout error + return nil, context.DeadlineExceeded + }) + }, + wantErr: "SilentError", + wantErrOut: heredoc.Doc(` + joel.miller + X joel.miller: timeout trying to connect to host + `), + }, { name: "hostname set", opts: &StatusOptions{ From 3bb62d4724b324bb5d6f0395aca93c9b9834cab8 Mon Sep 17 00:00:00 2001 From: nelsonchen304 <63536132+nelsonchen304@users.noreply.github.com> Date: Wed, 29 Nov 2023 17:29:26 +0800 Subject: [PATCH 4/4] Actions: filter to workflow runs for a specific commit (#8350) --- pkg/cmd/run/list/list.go | 3 +++ pkg/cmd/run/list/list_test.go | 18 ++++++++++++++++++ pkg/cmd/run/shared/shared.go | 4 ++++ 3 files changed, 25 insertions(+) diff --git a/pkg/cmd/run/list/list.go b/pkg/cmd/run/list/list.go index 7776b3521..0913babf3 100644 --- a/pkg/cmd/run/list/list.go +++ b/pkg/cmd/run/list/list.go @@ -34,6 +34,7 @@ type ListOptions struct { Status string Event string Created string + Commit string now time.Time } @@ -77,6 +78,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman cmd.Flags().StringVarP(&opts.Actor, "user", "u", "", "Filter runs by user who triggered the run") cmd.Flags().StringVarP(&opts.Event, "event", "e", "", "Filter runs by which `event` triggered the run") cmd.Flags().StringVarP(&opts.Created, "created", "", "", "Filter runs by the `date` it was created") + cmd.Flags().StringVarP(&opts.Commit, "commit", "c", "", "Filter runs by the `SHA` of the commit") cmdutil.StringEnumFlag(cmd, &opts.Status, "status", "s", "", shared.AllStatuses, "Filter runs by status") cmdutil.AddJSONFlags(cmd, &opts.Exporter, shared.RunFields) @@ -103,6 +105,7 @@ func listRun(opts *ListOptions) error { Status: opts.Status, Event: opts.Event, Created: opts.Created, + Commit: opts.Commit, } opts.IO.StartProgressIndicator() diff --git a/pkg/cmd/run/list/list_test.go b/pkg/cmd/run/list/list_test.go index 31680928f..1c3062804 100644 --- a/pkg/cmd/run/list/list_test.go +++ b/pkg/cmd/run/list/list_test.go @@ -479,6 +479,24 @@ func TestListRun(t *testing.T) { wantErr: true, wantErrMsg: "no runs found", }, + { + name: "commit filter applied", + opts: &ListOptions{ + Limit: defaultLimit, + Commit: "1234567890123456789012345678901234567890", + }, + isTTY: true, + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.QueryMatcher("GET", "repos/OWNER/REPO/actions/runs", url.Values{ + "head_sha": []string{"1234567890123456789012345678901234567890"}, + }), + httpmock.JSONResponse(shared.RunsPayload{}), + ) + }, + wantErr: true, + wantErrMsg: "no runs found", + }, } for _, tt := range tests { diff --git a/pkg/cmd/run/shared/shared.go b/pkg/cmd/run/shared/shared.go index 516e70b50..20846d60f 100644 --- a/pkg/cmd/run/shared/shared.go +++ b/pkg/cmd/run/shared/shared.go @@ -307,6 +307,7 @@ type FilterOptions struct { Status string Event string Created string + Commit string } // GetRunsWithFilter fetches 50 runs from the API and filters them in-memory @@ -358,6 +359,9 @@ func GetRuns(client *api.Client, repo ghrepo.Interface, opts *FilterOptions, lim if opts.Created != "" { path += fmt.Sprintf("&created=%s", url.QueryEscape(opts.Created)) } + if opts.Commit != "" { + path += fmt.Sprintf("&head_sha=%s", url.QueryEscape(opts.Commit)) + } } var result *RunsPayload