Upload artifact during CI #16633
Merged
Upload artifact during CI #16633
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
That is clever and could be such an improvement for reviewing the functionality of things! How will this keying scheme work when pushing a new commit causing the CI to rerun? Seems like they are immutable: https://github.com/actions/upload-artifact#breaking-changes |
Member
Author
the artifact namespace is unique per CI run, so the filename only needs to disambiguate between multiple artifact uploads within the same run (like how we have multiple jobs per platform within a single run) |
sholderbach
approved these changes
Sep 10, 2025
Member
sholderbach
left a comment
sholderbach
left a comment
There was a problem hiding this comment.
Good with landing this 🚀 let's test it in practice. (still need to check your snippet, especially with multiple artifacts per PR)
Member
Author
|
yea that was a quick thing, im planning on polishing it a bit (and maybe adding it to toolkit.nu?) |
Member
Author
|
i don't imagine anyone will object to this so i'll land it |
Contributor
|
nice! it would be good to have the script in the toolkit.nu in the root of the repo. |
132ikl
added a commit
that referenced
this pull request
Oct 7, 2025
This PR adds the `toolkit run pr` and `toolkit download pr` commands to `toolkit`. This is a more fleshed out version of the snippet shared in #16633, with robust error handling and cross-platform unzip support. When using `toolkit run pr`, the script will also check if the most recent binary for that PR has already been downloaded, and if so it will run that instead. I tried to make the error reporting as good as a built-in command to see how difficult that would be, and with use of the `--head: oneof<>` trick, it turned out pretty good. With access to the call span, the workflow is very similar to when writing a built-in command. I also used a `Spanned`-like record, which helped as well. ```nushell toolkit run pr 16740 # => Error: nu::shell::error # => # => × Command not found # => ╭─[entry #4:1:26] # => 1 │ overlay use -pr toolkit; toolkit run pr 16740 # => · ───────┬────── # => · ╰── requires `gh` # => ╰──── # => help: Please install the `gh` commandline tool ``` <details> <summary>More error reporting notes</summary> In an earlier version of the script, `run pr` called `download pr` directly. I ended up changing the way this worked so that `run pr` could use the workflow_id. Here's a couple snippets I thought were neat from this older version. **Passing span via `--head`:** ```nushell def download [--head: oneof<>] { let span = $head | default (metadata $head).span error make {msg: "a", label: {text: here, span: $span}} } def run [--head: oneof<>] { let span = (metadata $head).span download --head=$span } download # => Error: nu:🐚:error # => # => × a # => ╭─[entry #6:1:1] # => 1 │ download # => · ────┬─── # => · ╰── here # => ╰──── run # => Error: nu:🐚:error # => # => × a # => ╭─[entry #7:1:1] # => 1 │ run # => · ─┬─ # => · ╰── here # => ╰──── ``` **Using "spanned" number as CLI parameter and as internal caller parameter** ```nushell def download [number: oneof<int, record<item: int, span: record>>] { let number = match $number { {item: $_, span: $_} => $number, $val => {item: $number, span: (metadata $number).span} } error make {msg: "a", label: {text: here, span: $number.span}} } def run [number: int] { let number = {item: $number, span: (metadata $number).span} download $number } download 123 # => Error: nu:🐚:error # => # => × a # => ╭─[entry #9:1:10] # => 1 │ download 123 # => · ─┬─ # => · ╰── here # => ╰──── run 123 # => Error: nu:🐚:error # => # => × a # => ╭─[entry #10:1:5] # => 1 │ run 123 # => · ─┬─ # => · ╰── here # => ╰──── ``` </details> ## Release notes summary - What our users need to know The toolkit in the Nushell repository can now download and run PRs by downloading artifacts from CI runs. It can be run like this: ```nushell use toolkit toolkit run pr <number> ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR uploads the compiled Nushell binary as an artifact during the
std-lib-and-python-virtualenvtest. This makes it possible for reviewers to download the existing compiled binary from the CI rather than manually compiling it. A zip file containing the nushell binary is uploaded per platform, with this naming format:nu-<PR number or branch name>-platform.zipI put the retention policy to 14 days. I'm not sure how much this will impact our storage usage (cc @sholderbach) (I don't think I have permissions to see that?). I think ideally we would only keep the latest artifact per PR, but there isn't really any way to do that out of the box. If we need to implement due to storage space that then I can look into it.
As a bonus, here's a little script to run the latest artifact from a PR (if you'd like to try it, change
nushell/nushellto132ikl/nushelland dorun-pr 2:Release notes summary - What our users need to know
N/A