Thanks for the awesome CLI!
Describe the bug
The behaviour of the --name (-n) flag does not match the documented behaviour.
# Download specific artifacts across all runs in a repository
$ gh run download -n <name>
I would like to download all of the non-expired artifacts matching <name> from all workflows runs.
Expected vs actual behavior
$ gh run download -n test-reports
$ ls
test-run-25-0.log test-run-25-1.log test-run-25-2.log test-run-25-3.log
The result was 4 artifacts from workflow run with id 25. There are 22 other workflow runs with those artifacts not expired.
$curl -H "X-GitHub-Api-Version: 2022-11-28" https://api.github.com/repos/<owner>/<repo>/actions/artifacts?name=test-reports | \
jq '.artifacts[] | if .expired == false then .name else empty end' | uniq --count
23 "test-reports"
Analysis
The output above shows that every workflow run has the same name for the artifact. I think that's expected.
|
for _, a := range artifacts { |
|
if a.Expired { |
|
continue |
|
} |
|
if downloaded.Contains(a.Name) { |
|
continue |
|
} |
The downloaded.Contains line here causes the artifacts from all the previous workflows runs to be skipped. This prevents the download of artifacts from all previous workflow runs.
I believe this line comes from the original PR (c54e3c9#diff-e978a1b24757569c82622da106c0d0bee28cd927e627108f3c60a872f0aa6ec6R99-R101), before the --name flag was added.
I guess this condition is to avoid downloading the same artifact twice, if that is possible when glob patterns are used. Should it be
diff --git a/pkg/cmd/run/download/download.go b/pkg/cmd/run/download/download.go
index 25eec5d62..eaf2eae7c 100644
--- a/pkg/cmd/run/download/download.go
+++ b/pkg/cmd/run/download/download.go
@@ -151,7 +151,7 @@ func runDownload(opts *DownloadOptions) error {
if a.Expired {
continue
}
- if downloaded.Contains(a.Name) {
+ if downloaded.Contains(a.DownloadURL) {
continue
}
if len(wantNames) > 0 || len(wantPatterns) > 0 {
or
diff --git a/pkg/cmd/run/download/download.go b/pkg/cmd/run/download/download.go
index 25eec5d62..cdc8f6b83 100644
--- a/pkg/cmd/run/download/download.go
+++ b/pkg/cmd/run/download/download.go
@@ -151,7 +151,7 @@ func runDownload(opts *DownloadOptions) error {
if a.Expired {
continue
}
- if downloaded.Contains(a.Name) {
+ if len(opts.Names) == 0 && downloaded.Contains(a.Name) {
continue
}
if len(wantNames) > 0 || len(wantPatterns) > 0 {
I think either of those would fix the bug and provide the documented behaviour. A new test case in Test_runDownload should be easy to add, if you would accept a PR for one of those fixes.
Thanks for the awesome CLI!
Describe the bug
The behaviour of the
--name(-n) flag does not match the documented behaviour.I would like to download all of the non-expired artifacts matching
<name>from all workflows runs.Expected vs actual behavior
The result was 4 artifacts from workflow run with id 25. There are 22 other workflow runs with those artifacts not expired.
Analysis
The output above shows that every workflow run has the same name for the artifact. I think that's expected.
cli/pkg/cmd/run/download/download.go
Lines 150 to 156 in 1fbcdf5
The
downloaded.Containsline here causes the artifacts from all the previous workflows runs to be skipped. This prevents the download of artifacts from all previous workflow runs.I believe this line comes from the original PR (c54e3c9#diff-e978a1b24757569c82622da106c0d0bee28cd927e627108f3c60a872f0aa6ec6R99-R101), before the
--nameflag was added.I guess this condition is to avoid downloading the same artifact twice, if that is possible when glob patterns are used. Should it be
or
I think either of those would fix the bug and provide the documented behaviour. A new test case in
Test_runDownloadshould be easy to add, if you would accept a PR for one of those fixes.