fix gh run download <run-id> not getting > 100 artifacts (#5799)
Co-authored-by: Mislav Marohnić <mislav@github.com>
This commit is contained in:
parent
dea1af1227
commit
11b1059237
2 changed files with 111 additions and 12 deletions
|
|
@ -4,6 +4,7 @@ import (
|
|||
"encoding/json"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"regexp"
|
||||
|
||||
"github.com/cli/cli/v2/api"
|
||||
"github.com/cli/cli/v2/internal/ghinstance"
|
||||
|
|
@ -17,37 +18,69 @@ type Artifact struct {
|
|||
Expired bool `json:"expired"`
|
||||
}
|
||||
|
||||
type artifactsPayload struct {
|
||||
Artifacts []Artifact
|
||||
}
|
||||
|
||||
func ListArtifacts(httpClient *http.Client, repo ghrepo.Interface, runID string) ([]Artifact, error) {
|
||||
var results []Artifact
|
||||
|
||||
perPage := 100
|
||||
path := fmt.Sprintf("repos/%s/%s/actions/artifacts?per_page=%d", repo.RepoOwner(), repo.RepoName(), perPage)
|
||||
if runID != "" {
|
||||
path = fmt.Sprintf("repos/%s/%s/actions/runs/%s/artifacts?per_page=%d", repo.RepoOwner(), repo.RepoName(), runID, perPage)
|
||||
}
|
||||
|
||||
req, err := http.NewRequest("GET", ghinstance.RESTPrefix(repo.RepoHost())+path, nil)
|
||||
url := fmt.Sprintf("%s%s", ghinstance.RESTPrefix(repo.RepoHost()), path)
|
||||
|
||||
for {
|
||||
var payload artifactsPayload
|
||||
nextURL, err := apiGet(httpClient, url, &payload)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
results = append(results, payload.Artifacts...)
|
||||
|
||||
if nextURL == "" {
|
||||
break
|
||||
}
|
||||
url = nextURL
|
||||
}
|
||||
|
||||
return results, nil
|
||||
}
|
||||
|
||||
func apiGet(httpClient *http.Client, url string, data interface{}) (string, error) {
|
||||
req, err := http.NewRequest("GET", url, nil)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
return "", err
|
||||
}
|
||||
|
||||
resp, err := httpClient.Do(req)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
return "", err
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
if resp.StatusCode > 299 {
|
||||
return nil, api.HandleHTTPError(resp)
|
||||
}
|
||||
|
||||
var response struct {
|
||||
TotalCount uint16 `json:"total_count"`
|
||||
Artifacts []Artifact
|
||||
return "", api.HandleHTTPError(resp)
|
||||
}
|
||||
|
||||
dec := json.NewDecoder(resp.Body)
|
||||
if err := dec.Decode(&response); err != nil {
|
||||
return response.Artifacts, fmt.Errorf("error parsing JSON: %w", err)
|
||||
if err := dec.Decode(data); err != nil {
|
||||
return "", err
|
||||
}
|
||||
|
||||
return response.Artifacts, nil
|
||||
return findNextPage(resp), nil
|
||||
}
|
||||
|
||||
var linkRE = regexp.MustCompile(`<([^>]+)>;\s*rel="([^"]+)"`)
|
||||
|
||||
func findNextPage(resp *http.Response) string {
|
||||
for _, m := range linkRE.FindAllStringSubmatch(resp.Header.Get("Link"), -1) {
|
||||
if len(m) > 2 && m[2] == "next" {
|
||||
return m[1]
|
||||
}
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
|
|
|||
66
pkg/cmd/run/shared/artifacts_test.go
Normal file
66
pkg/cmd/run/shared/artifacts_test.go
Normal file
|
|
@ -0,0 +1,66 @@
|
|||
package shared
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"testing"
|
||||
|
||||
"github.com/cli/cli/v2/internal/ghrepo"
|
||||
"github.com/cli/cli/v2/pkg/httpmock"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestDownloadWorkflowArtifactsPageinates(t *testing.T) {
|
||||
testRepoOwner := "OWNER"
|
||||
testRepoName := "REPO"
|
||||
testRunId := "1234567890"
|
||||
|
||||
reg := &httpmock.Registry{}
|
||||
defer reg.Verify(t)
|
||||
|
||||
firstReq := httpmock.QueryMatcher(
|
||||
"GET",
|
||||
fmt.Sprintf("repos/%s/%s/actions/runs/%s/artifacts", testRepoOwner, testRepoName, testRunId),
|
||||
url.Values{"per_page": []string{"100"}},
|
||||
)
|
||||
|
||||
firstArtifact := Artifact{
|
||||
Name: "artifact-0",
|
||||
Size: 2,
|
||||
DownloadURL: fmt.Sprintf("https://api.github.com/repos/%s/%s/actions/artifacts/987654320/zip", testRepoOwner, testRepoName),
|
||||
Expired: true,
|
||||
}
|
||||
firstRes := httpmock.JSONResponse(artifactsPayload{Artifacts: []Artifact{firstArtifact}})
|
||||
testLinkUri := fmt.Sprintf("repositories/123456789/actions/runs/%s/artifacts", testRunId)
|
||||
testLinkUrl := fmt.Sprintf("https://api.github.com/%s", testLinkUri)
|
||||
firstRes = httpmock.WithHeader(
|
||||
firstRes,
|
||||
"Link",
|
||||
fmt.Sprintf(`<%s?per_page=100&page=2>; rel="next", <%s?per_page=100&page=2>; rel="last"`, testLinkUrl, testLinkUrl),
|
||||
)
|
||||
|
||||
secondReq := httpmock.QueryMatcher(
|
||||
"GET",
|
||||
testLinkUri,
|
||||
url.Values{"per_page": []string{"100"}, "page": []string{"2"}},
|
||||
)
|
||||
|
||||
secondArtifact := Artifact{
|
||||
Name: "artifact-1",
|
||||
Size: 2,
|
||||
DownloadURL: fmt.Sprintf("https://api.github.com/repos/%s/%s/actions/artifacts/987654321/zip", testRepoOwner, testRepoName),
|
||||
Expired: false,
|
||||
}
|
||||
secondRes := httpmock.JSONResponse(artifactsPayload{Artifacts: []Artifact{secondArtifact}})
|
||||
|
||||
reg.Register(firstReq, firstRes)
|
||||
reg.Register(secondReq, secondRes)
|
||||
|
||||
httpClient := &http.Client{Transport: reg}
|
||||
repo := ghrepo.New(testRepoOwner, testRepoName)
|
||||
|
||||
result, err := ListArtifacts(httpClient, repo, testRunId)
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, []Artifact{firstArtifact, secondArtifact}, result)
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue