Bump the actions group with 6 updates #29203
Conversation
|
I was kind of expecting
|
Bumps the actions group with 6 updates: | Package | From | To | | --- | --- | --- | | [actions/checkout](https://github.com/actions/checkout) | `3` | `4` | | [actions/setup-python](https://github.com/actions/setup-python) | `4` | `5` | | [actions/cache](https://github.com/actions/cache) | `3` | `4` | | [actions/upload-artifact](https://github.com/actions/upload-artifact) | `3` | `4` | | [actions/download-artifact](https://github.com/actions/download-artifact) | `3` | `4` | | [peter-evans/create-pull-request](https://github.com/peter-evans/create-pull-request) | `5` | `6` | Updates `actions/checkout` from 3 to 4 - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v3...v4) Updates `actions/setup-python` from 4 to 5 - [Release notes](https://github.com/actions/setup-python/releases) - [Commits](actions/setup-python@v4...v5) Updates `actions/cache` from 3 to 4 - [Release notes](https://github.com/actions/cache/releases) - [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md) - [Commits](actions/cache@v3...v4) Updates `actions/upload-artifact` from 3 to 4 - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@v3...v4) Updates `actions/download-artifact` from 3 to 4 - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](actions/download-artifact@v3...v4) Updates `peter-evans/create-pull-request` from 5 to 6 - [Release notes](https://github.com/peter-evans/create-pull-request/releases) - [Commits](peter-evans/create-pull-request@v5...v6) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions - dependency-name: actions/setup-python dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions - dependency-name: actions/cache dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions - dependency-name: actions/upload-artifact dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions - dependency-name: actions/download-artifact dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions - dependency-name: peter-evans/create-pull-request dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions ... Signed-off-by: dependabot[bot] <support@github.com>
25f7e31 to
11857ea
Compare
I double-checked in a test repo and it seems this is what is happening indeed. Dependabot respects what is already used in the workflow files:
I would be in favour of using hashes with human readable comment everywhere, any objections? i.e.: uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # v3.6.0instead of what we are currently doing: uses: actions/checkout@v3If no objection I will open a PR that does that. |
|
+1 to using the minor versions with hashes for the publishing steps. I remember that there was some subtly to the upload / download actions on the latest versions, but I will need to look those up when I'm not in my phone. @henryiii might remember off the top of his head though. |
I have opened #29206 to pin Github Actions with commit hashes. |
I think this was upload-artifacts v4 that had some behaviour changes https://github.blog/2024-02-12-get-started-with-v4-of-github-actions-artifacts/#compatibility for example:
For now my PR #29206 is trying to keep the same versions that we were using. I guess we will need to be careful indeed before merging this Dependabot PR. Given that upload-artifact is only used in |
|
Using exact pins causes a lot more churn and restricts you from getting the latest patch versions, which can fix things. GitHub Actions is not a fixed platform anyway, the runner images are updated regularly, so pinning (official) actions is generally less helpful than it's worth, IMO. Pinning it in the publish step is a good idea, but the rest will likely mean you'll get weekly (or whatever) updates of hashes with little benefit. I'd also make sure it updates frequently if you don't allow patch fixes. Upload/download v4 require each other, and they no longer auto-merge if you reuse the same artifact name. All artifacts must now be uploaded once with unique names. You also need to make sure you don't upload the same file if you download and merge (new features for merging multiple artifacts added to help). (By the way, in case it's not clear, my comment is about pinning the official actions like actions/checkout. It is less applicable to third party actions.)\ (Node versions, runner images, the python-versions cache setup-python pulls from, etc. - All this is not pinned, so pinning the action simply doesn't improve reliability) |
|
Upload / download artifact v3 will stop working November 30, 2024, FYI. Another example of why pinning official actions isn't really something you fully can control. |
|
Thanks @matthewfeickert and @henryiii for your comments. To make sure I am following:
|
|
I think yes to the first two, and the third, it's up to you. I tend to not worry about thirty party actions if they provide a moving major tag, I assume they will also keep it working, and I like lower CI churn, so I'm very slightly on the major tag for everyone side. I don't think I've run into a problem yet. But it's a trade off and it depends on your preference; pinning means you don't have to trust the third party actions as much, which does sound nice. If you are worried about security anywhere, go ahead and pin - that's why the publish step is pinned. |
| @@ -215,7 +215,7 @@ jobs: | |||
| SKLEARN_SKIP_NETWORK_TESTS: 1 | |||
|
|
|||
| - name: Store artifacts | |||
| uses: actions/upload-artifact@v3 | |||
| uses: actions/upload-artifact@v4 | |||
| with: | |||
| path: dist/*.tar.gz | |||
There was a problem hiding this comment.
I'd recommend naming the artifact here; using the default name is tricky now since you can only do it one time.
| @@ -175,7 +175,7 @@ jobs: | |||
| run: bash build_tools/wheels/build_wheels.sh | |||
|
|
|||
| - name: Store artifacts | |||
| uses: actions/upload-artifact@v3 | |||
| uses: actions/upload-artifact@v4 | |||
| with: | |||
| path: wheelhouse/*.whl | |||
There was a problem hiding this comment.
This needs a unique name per job, and then the download step needs to merge. See https://learn.scientific-python.org/development/guides/gha-wheels for example.
There was a problem hiding this comment.
I agree with all of @henryiii's suggested changes (no surprise there) but to answer @lesteve's question (sorry for delay, am traveling today):
- we already pin with hashes in the publish steps
- using only major versions for official Github Actions (like actions/checkout and others) seems better
- third-party actions, I am not so sure now ...
- SPEC 8 is just recommending pinning for security focused steps like publishing. In PR CI Enable Dependabot and use full length commit SHA for PyPI publishing #29180 I just did the upload step as that's a big step forward, but after SPEC 8 is ratified we can open up PRs to ping other actions in publishing workflows.
- Yes, for thing like
actions/whateverjust use the major version. - I think it is fine to use the major version for third parties, but if you want to pin to the patch level you'll only get a once a month PR so that's not too bad. But this is very much a "what do you want to do" situation.
|
I think one month is too long if you pin to patch versions, I'd only recommend that for looser pins. As an example, the runner images updated powershell a while back. This broke all existing versions of the cibuildwheel action, and we had to rush v2.16.5 out to fix all Windows users of the action. If you had cibuildwheel pinned to anything before that, you were broken by GitHub. However, if you pinned to our moving tag (v2.16), you were fine (unless you got lucky in the <2 day window where we pushed out the fix). |
|
(On phone again) @henryiii This will be relevant for discussion on the follow up SPEC to SPEC 8 on security for building artifacts. I imagine that this will be something that Seth will want to discuss more broadly as well. I'll defer to the scikit-learn team on if they are fine to bump to weekly over monthly for Dependabot, though @henryiii I thought you were going to recommend switching to monthly in the Scientific Python Developer Guide. I will read again once I land tonight as I might be reading quickly and missing context. |
|
I did recommend switching to monthly, but that's with pinning to major versions. If you are isolating yourself from getting patch releases, you need to update more often. Personally, I think this is generally fine, lower churn and keeping CI simple is better than complexity and frequent updates, but if that level of security is needed for certain applications, that's fine for those applications. If doing that, you should also be locking your dependencies with hashes, etc, which we also don't do in the dev guide, as that slows your response time if dependencies break (among other things). |
|
OK thanks for your feed-back @matthewfeickert @henryiii, I have a better understanding of the trade-offs now. I think for now we should stick to major versions (official Github actions + third-party actions) for now. Let's wait for SPEC 8 to settle down and potentially some PRs from people working on SPEC 8, to see what else can be improved. I have opened a separate PR #29211 to do the Side-comment: the "Pull Request Regex Title Labeler / labeler (pull_request_target)" CI failure probably happens because we pass the |
|
Shall we close this PR in favor of #29211 and the wait for the next denpendabot run to update the other (non-artifact related) actions? |
I think we can leave the Dependabot PR open. I think the Dependabot PR will either be updated automatically because I guess one way to find out is to try 😉, i.e. merging #29211 first while leaving the Dependabot PR open. |
|
Looks like these dependencies are updatable in another way, so this is no longer needed. |
|
Now over at PR #29214 |
Bumps the actions group with 6 updates:
344534343456Updates
actions/checkoutfrom 3 to 4Release notes
Sourced from actions/checkout's releases.
... (truncated)
Changelog
Sourced from actions/checkout's changelog.
... (truncated)
Commits
a5ac7e5Update for 4.1.6 release (#1733)24ed1a3Check platform for extension (#1732)44c2b7aREADME: Suggestuser.emailto be `41898282+github-actions[bot]@users.norepl...8459bc0Bump actions/upload-artifact from 2 to 4 (#1695)3f603f6Bump actions/setup-node from 1 to 4 (#1696)fd084cdBump github/codeql-action from 2 to 3 (#1694)9c1e94eUpdate NPM dependencies (#1703)0ad4b8fPrep Release v4.1.4 (#1704)43045aeDisableextensions.worktreeConfigwhen disablingsparse-checkout(#1692)37b0821Bump the minor-actions-dependencies group with 2 updates (#1693)Updates
actions/setup-pythonfrom 4 to 5Release notes
Sourced from actions/setup-python's releases.
... (truncated)
Commits
82c7e63Documentation changes for avoiding rate limit issues on GHES (#835)10aa35afeat: fallback to raw endpoint for manifest when rate limit is reached (#766)9a7ac94Bump undici from 5.27.2 to 5.28.3 (#817)871daa9Fix the "Specifying multiple Python/PyPy versions" link (#782)2f07895Fix broken README.md link (#793)e9d6f99Replace setup-python@v4 by setup-python@v5 in README (#776)0a5c615Update action to node20 (#772)0ae5836Add example of GraalPy to docs (#773)b64ffcaupdate actions/checkout to v4 (#761)8d28961Examples now use checkout@v4 (#738)Updates
actions/cachefrom 3 to 4Release notes
Sourced from actions/cache's releases.
... (truncated)
Changelog
Sourced from actions/cache's changelog.
... (truncated)
Commits
0c45773Merge pull request #1327 from cdce8p/fix-fail-on-cache-miss8a55f83Add test case for process exit3884cacBump versione29dad3Fix fail-on-cache-miss not workingab5e6d0Merge pull request #1341 from bethanyj28/main89c7d86licensed cached2c84daupdate@actions/cache37e7d4eMerge pull request #1340 from actions/bethanyj28/update-publish-flowa18323fadd release actiona2ed59dMerge pull request #1305 from actions/yacaovsnc/update_examplesUpdates
actions/upload-artifactfrom 3 to 4Release notes
Sourced from actions/upload-artifact's releases.
Commits
6546280updating package versionc004fb4Merge branch 'main' into eggyhead/use-artifact-v2.1.690aba49updating toolkit artifact dependency to 2.1.6b06cde3Merge pull request #563 from actions/eggyhead/release-4.3.21746f4aRevert "updating to release 4.3.2"31685d0updating to release 4.3.218bf333Merge pull request #562 from actions/eggyhead/update-artifact-v215dac413bupdate package lock versionbb3b4a3updating package version3e3da83updating artifact and core dependenciesUpdates
actions/download-artifactfrom 3 to 4Release notes
Sourced from actions/download-artifact's releases.
Commits
65a9edcMerge pull request #325 from bethanyj28/mainfdd1595licensedc13dba1update@actions/artifactdependency0daa75eMerge pull request #324 from actions/eggyhead/use-artifact-v2.1.69c19ed7Merge branch 'main' into eggyhead/use-artifact-v2.1.63d3ea87updating license89af5dbupdating artifact package v2.1.6b4aefffMerge pull request #323 from actions/eggyhead/update-artifact-v2158caf195package lock updated7a2ec4updating package versionUpdates
peter-evans/create-pull-requestfrom 5 to 6Release notes
Sourced from peter-evans/create-pull-request's releases.
Commits
6d6857dfix: update proxy support to follow octokit change to fetch api (#2867)9153d83perf: limit the fetch depth of pr branch (#2857)c55203cfix: drop unnecessary fetch with unshallow on push-to-fork (#2849)6ce4ecabuild(deps-dev): bump@types/nodefrom 18.19.28 to 18.19.31 (#2842)36ef0edbuild(deps-dev): bump@types/nodefrom 18.19.26 to 18.19.28 (#2836)8500972build(deps-dev): bump@types/nodefrom 18.19.25 to 18.19.26 (#2831)bda5adebuild(deps-dev): bump@types/nodefrom 18.19.23 to 18.19.25 (#2826)70a41abperf: shallow fetch the actual base when rebasing from working base (#2816)57a1014build(deps-dev): bump@types/nodefrom 18.19.21 to 18.19.23 (#2811)b3a2c5dbuild(deps-dev): bump@types/nodefrom 18.19.18 to 18.19.21 (#2798)Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot mergewill merge this PR after your CI passes on it@dependabot squash and mergewill squash and merge this PR after your CI passes on it@dependabot cancel mergewill cancel a previously requested merge and block automerging@dependabot reopenwill reopen this PR if it is closed@dependabot closewill close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore <dependency name> major versionwill close this group update PR and stop Dependabot creating any more for the specific dependency's major version (unless you unignore this specific dependency's major version or upgrade to it yourself)@dependabot ignore <dependency name> minor versionwill close this group update PR and stop Dependabot creating any more for the specific dependency's minor version (unless you unignore this specific dependency's minor version or upgrade to it yourself)@dependabot ignore <dependency name>will close this group update PR and stop Dependabot creating any more for the specific dependency (unless you unignore this specific dependency or upgrade to it yourself)@dependabot unignore <dependency name>will remove all of the ignore conditions of the specified dependency@dependabot unignore <dependency name> <ignore condition>will remove the ignore condition of the specified dependency and ignore conditions