Skip to content

Use unique artifact identifier in upload-artifact step #648

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.

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

Merged
merged 6 commits into from
Aug 6, 2024

Conversation

jprinet
Copy link
Member

@jprinet jprinet commented Aug 5, 2024

Issue

Fixes #644

Since upload-artifact@v4, uploading again the same artifact name in a workflow fails the step.
Using a hardcoded experiment-X-receipt is not valid anymore

Fix

Use a unique name. This is not so straightforward as some collisions can occur in the case of matrix.
Using the job-index for matrix solves this.
Note that appending the job-index is happening only if job-total > 1 to not suffix artifacts from non matrix scenario

This could hopefully get simplified if/when GitHub starts to expose a unique job-id when running with matrix strategy.

Extra

A receipt download link has been added as part of the workflow summary

Example

See a summary running with the PR fix here

@jprinet jprinet requested a review from erichaagdev August 5, 2024 13:33
@jprinet jprinet force-pushed the jprinet/fix-unique-artifact-id-violation branch from 54820bc to b35a120 Compare August 5, 2024 14:51
@jprinet jprinet marked this pull request as draft August 5, 2024 14:52
@jprinet jprinet marked this pull request as ready for review August 5, 2024 16:03
@wbonnefond
Copy link

This branch fixes the issue I was having in our codebase

@jprinet jprinet force-pushed the jprinet/fix-unique-artifact-id-violation branch 4 times, most recently from 3356a86 to 560844a Compare August 6, 2024 13:21
- name: Add artifact link to summary
run: |
echo "-------------" >> $GITHUB_STEP_SUMMARY
echo "Download receipt: [${{ steps.upload-artifact.outputs.artifact-url }}](${{ steps.upload-artifact.outputs.artifact-url }})" >> $GITHUB_STEP_SUMMARY
Copy link
Member

Choose a reason for hiding this comment

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

It's already a link. Is it necessary to wrap it like this?

Suggested change
echo "Download receipt: [${{ steps.upload-artifact.outputs.artifact-url }}](${{ steps.upload-artifact.outputs.artifact-url }})" >> $GITHUB_STEP_SUMMARY
echo "Download receipt: ${{ steps.upload-artifact.outputs.artifact-url }}" >> $GITHUB_STEP_SUMMARY

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point 😅
I adjusted the PR

@jprinet jprinet force-pushed the jprinet/fix-unique-artifact-id-violation branch from 560844a to 22da139 Compare August 6, 2024 14:22
Copy link
Member

@erichaagdev erichaagdev left a comment

Choose a reason for hiding this comment

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

🚀

@jprinet jprinet merged commit 60a34f2 into main Aug 6, 2024
5 checks passed
@jprinet jprinet deleted the jprinet/fix-unique-artifact-id-violation branch August 6, 2024 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Artifact name for 'Archive receipt' should be configurable
3 participants