Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

{pre,post}-build: use actions/{upload,download}-artifact@v4 #529

Merged
merged 6 commits into from
Apr 18, 2024
Merged

{pre,post}-build: use actions/{upload,download}-artifact@v4 #529

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f110e93
pre-build: use `actions/download-artifact@v4`
ZhongRuoyu Apr 16, 2024
d174d1e
post-build: use `actions/upload-artifact@v4`
ZhongRuoyu Apr 16, 2024
958e8f0
post-build: update artifact name for failed bottles
ZhongRuoyu Apr 16, 2024
e4c7508
{pre,post}-build: update artifact patterns
ZhongRuoyu Apr 16, 2024
3a374c3
pre-build: simplify pattern
ZhongRuoyu Apr 16, 2024
862483c
post-build: use `_${{ inputs.runner }}` in artifact names
ZhongRuoyu Apr 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 7 additions & 7 deletions post-build/action.yml
View file
Open in desktop
Expand Up @@ -49,7 +49,7 @@ runs:

- name: Upload logs
if: always()
uses: actions/upload-artifact@v3-node20
uses: actions/upload-artifact@v4
with:
name: logs-${{ inputs.runner }}
path: ${{ inputs.logs-directory }}
Expand All @@ -63,16 +63,16 @@ runs:

- name: Upload failed bottles
if: always() && steps.bottles.outputs.failures > 0
uses: actions/upload-artifact@v3-node20
uses: actions/upload-artifact@v4
with:
name: bottles-${{ inputs.runner }}
name: failed-bottles-${{ inputs.runner }}
path: ${{ inputs.bottles-directory }}/failed

- name: Upload list of successfully tested dependents
if: always() && !fromJson(inputs.upload-bottles) && steps.bottles.outputs.dependents > 0
uses: actions/upload-artifact@v3-node20
uses: actions/upload-artifact@v4
with:
name: dependents
name: dependents-${{ inputs.runner }}
ZhongRuoyu marked this conversation as resolved.
Show resolved Hide resolved
path: ${{ inputs.bottles-directory }}/tested-dependents-*.txt

# Must be run before the `Upload bottles` step so that failed
Expand All @@ -87,9 +87,9 @@ runs:

- name: Upload bottles
if: always() && fromJson(inputs.upload-bottles) && (steps.bottles.outputs.count > 0 || steps.bottles.outputs.skipped > 0)
uses: actions/upload-artifact@v3-node20
uses: actions/upload-artifact@v4
with:
name: bottles
name: bottles_${{ inputs.runner }}
path: ${{ inputs.bottles-directory }}

- name: Post cleanup
Expand Down
5 changes: 3 additions & 2 deletions pre-build/action.yml
View file
Open in desktop
Expand Up @@ -67,7 +67,8 @@ runs:

- name: Download bottles from GitHub Actions
if: fromJson(inputs.download-bottles)
uses: actions/download-artifact@v3-node20
uses: actions/download-artifact@v4
with:
name: bottles
pattern: bottles{,_*}
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
pattern: bottles{,_*}
pattern: bottles_*

Because v4 cannot download v3 artifacts anyway, according to https://github.com/actions/download-artifact/tree/v4/#v4---whats-new.

This affects dependent testing, which means some of the ongoing CI jobs will fail when this is merged. Preferably the long timeout jobs should be at dep testing (i.e., after bottles are downloaded) when merging this.

Copy link
Member

@Bo98 Bo98 Apr 17, 2024 •
edited

Choose a reason for hiding this comment

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

pr-pull uses REST API which wraps v3 and v4 that part is probably OK at least.

But yeah, for dep testing it's perhaps best to time this for when the CI queue is empty. I can also watch out for that opportunity similarly if needed.

path: ${{ inputs.bottles-directory }}
merge-multiple: true