Skip to content

Conversation

@mislav
Copy link
Contributor

@mislav mislav commented Apr 2, 2021

Downloading artifacts per run: gh run download 123

  • downloads all non-expired artifacts from a run
  • optionally, specific artifacts can be selected using the -p flag

Downloading artifacts per repository: gh run download -p name1 -p name2

  • downloads matching non-expired artifacts across all runs
  • specifying the name pattern is required in this case

No arguments = error. Should we display some kind of selector in this case? If so, what should the options in the selector be?

Inspirations:

TODO:

  • tests for downloading & zip extraction
  • when downloading multiple artifacts, organize extracted files in sub-directories based on artifact name
  • address CodeQL warning

Fixes #3321

Copy link
Contributor

@vilmibm vilmibm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm mostly happy with the usability and direction of this! I otherwise just have questions around testing and code organization.

"github.com/cli/cli/api"
"github.com/cli/cli/internal/ghinstance"
"github.com/cli/cli/internal/ghrepo"
"github.com/cli/cli/pkg/cmd/run/download"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this dependency be done via a shared package instead of a direct subcommand dependency?

reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3"),
httpmock.JSONResponse(shared.SuccessfulRun))
reg.Register(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems worth testing the artifact printing in at least one case

@vilmibm vilmibm mentioned this pull request Apr 5, 2021
31 tasks
@vilmibm vilmibm added the actions label Apr 5, 2021
mislav added 5 commits April 7, 2021 19:55
- With no arguments in TTY mode, prompt which artifacts to download
- Change `--pattern` argument to be just `--name` and only do exact
  matching
- For multi-archive downloads, prefix the destination path with the name
  of the artifact
- Add tests exercising HTTP functionality
- Avoid "zipslip" path injection when extracting ZIP files
- Add tests for ZIP extraction
@mislav mislav force-pushed the artifact-download branch from 78a7a6b to 026dc46 Compare April 7, 2021 18:26
@mislav mislav requested a review from vilmibm April 7, 2021 18:26
Copy link
Contributor

@vilmibm vilmibm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiniest nitpick about Examples and it needs a fix for the Windows tests but otherwise this is great!

@vilmibm vilmibm enabled auto-merge April 7, 2021 19:49
@vilmibm vilmibm merged commit 35c55fd into trunk Apr 7, 2021
@mislav mislav deleted the artifact-download branch April 8, 2021 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

artifact support for gh run

3 participants