-
Notifications
You must be signed in to change notification settings - Fork 18
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
Upgrade to v4 download and upload artifacts #550
Upgrade to v4 download and upload artifacts #550
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In dataverse project we had to use v4.1.7. Is there a reason not to use this instead of v4?
# using v4.1.7 due to a bug in v4
uses: actions/[email protected]
Hi @stevenwinship, thanks! I thought using only v4 would always point to the latest version 4, but it might be safer to use an exact version, could you also tell me what exact version you are using for the upload-artifact? |
It's very confusing. https://github.com/orgs/community/discussions/25248#discussioncomment-3247083 by @ethomson (hi, Edward! 👋 ) says, "we only match a literal release tag". However the GitHub Actions Toolkit docs at https://github.com/actions/toolkit/blob/master/docs/action-versioning.md#versioning say, "Binding to a major version is the latest of that major version ( e.g. v1 == "1.*" )" Then it goes on to say this: "Make the new release available to those binding to the major version tag: Move the major version tag (v1, v2, etc.) to point to the ref of the current release." So it might depend on if people are using GitHub Actions Toolkit or not? I'm not sure. |
Hey @pdurbin - that's correct; actions authors need to make the updates to the "major version tag" themselves. GitHub has also started suggesting to people that they pin to a SHA instead of referencing a tag, for security. https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions# There's a usability gap here, though. Coincidentally, I work on a tool that can help - https://github.com/stacklok/frizbee will take care of the annoying SHA pinning for you. |
@ethomson thanks for getting back to us so fast! Does this mean that for any action we can't know if the author is making updates to the "major version tag" or not? We have to poke around? For example, when I look at the "download-artifact" action, I'm (now) fairly confident that "v4" means "latest 4" by looking at the following:
|
For upload-artifact we are using v4 |
I think that you do. And there's a few cases here: the author does this always, and reliably, the author never does this, or the author has a manual process to do this when they publish and maybe sometimes forgets. I think that GitHub is pretty good about always updating the major version tag for their actions. (Hopefully automated, or at least tested; I honestly don't remember.) But me, personally, I'm in the latter camp and sometimes forget. 😢 |
It looks like test / component (pull_request) is failing - https://github.com/IQSS/dataverse-frontend/actions/runs/11862443295/job/33392252664?pr=550 |
@ofahimIQSS all tests are passing now, there were some missing props in a unit test component. 👍🏼 , Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changes look good
What this PR does / why we need it:
Updates download and upload artifacts to version 4.
upload-artifacts have been updated in the following workflows:
download-artifacts have been updated in the following workflows:
Which issue(s) this PR closes:
Suggestions on how to test this:
Visual code inspection and re-run the test action.
deploy.yml
workflow has not been used.deploy-beta-testing.yml
workflow has been already tested deploying this current branch and it can be checked again after merging this branch into develop.Note: the code coverage (coverall report) says it has decreased but it is the same rare problem that keeps occurring sometimes, we should not worry about it on this issue.
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
No
Is there a release notes update needed for this change?:
No
Additional documentation:
No