-
Notifications
You must be signed in to change notification settings - Fork 557
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 upload-artifact and download-artifact actions to v4 #4518
Conversation
Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 3 to 4. - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](actions/download-artifact@v3...v4) --- updated-dependencies: - dependency-name: actions/download-artifact dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 3 to 4. - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](actions/download-artifact@v3...v4) --- updated-dependencies: - dependency-name: actions/download-artifact dependency-type: direct:production update-type: version-update:semver-major ... PR tests below
WalkthroughThe recent updates primarily focus on enhancing the GitHub Actions workflows by upgrading the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (5)
- .github/workflows/build-db.yml (2 hunks)
- .github/workflows/build-desktop.yml (1 hunks)
- .github/workflows/build-docs.yml (1 hunks)
- .github/workflows/build-graphql.yml (1 hunks)
- .github/workflows/publish.yml (2 hunks)
Files skipped from review due to trivial changes (5)
- .github/workflows/build-db.yml
- .github/workflows/build-desktop.yml
- .github/workflows/build-docs.yml
- .github/workflows/build-graphql.yml
- .github/workflows/publish.yml
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.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (7)
- .github/workflows/build-db.yml (3 hunks)
- .github/workflows/build-desktop.yml (2 hunks)
- .github/workflows/build-docs.yml (2 hunks)
- .github/workflows/build-graphql.yml (2 hunks)
- .github/workflows/build.yml (1 hunks)
- .github/workflows/e2e.yml (1 hunks)
- .github/workflows/publish.yml (2 hunks)
Files skipped from review due to trivial changes (2)
- .github/workflows/build.yml
- .github/workflows/e2e.yml
Files skipped from review as they are similar to previous changes (5)
- .github/workflows/build-db.yml
- .github/workflows/build-desktop.yml
- .github/workflows/build-docs.yml
- .github/workflows/build-graphql.yml
- .github/workflows/publish.yml
This change isn't going to work, the source files aren't included in the distribution - this is likely to require a bigger refactor to get the upgrades to work... |
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.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/build-db.yml (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/build-db.yml
67d7d20
to
036533f
Compare
.github/workflows/build-db.yml
Outdated
@@ -44,37 +44,35 @@ jobs: | |||
run: | | |||
cd package/db | |||
FODB_MACHINE=arm64 FODB_SYSTEM=Darwin python -Im build | |||
python -Im build --sdist |
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.
Look, I don't like this answer either, but with the new v4 'immutable artifacts' rule I don't have any other choices to create an artifact that contains the wheel and the source file...
I will happily entertain other options if you have them!
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.
My assumption that the artifacts need to contain the source tar.gz
and the whl are based on the artifacts in this workflow - if my assumption is wrong, that'd be swell, but 🤷
This doesn't slow down the overall tests at all, the build-db.yml
workflow still finishes under 2 minutes (with comparable times from 1m42s to 2m1s) and this isn't the long pole in the tent
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.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/build-db.yml (3 hunks)
Additional comments not posted (3)
.github/workflows/build-db.yml (3)
107-107
: Confirm that theactions/download-artifact@v4
correctly downloads all required artifacts for the publish job without any missing or duplicate files.
72-72
: Ensure that theactions/upload-artifact@v4
is compatible with all platforms specified in the matrix.Verification successful
The verification process did not yield any specific compatibility issues with the
actions/upload-artifact@v4
action across different platforms. The only relevant information found was thatupload-artifact@v4
is not currently supported on GitHub Enterprise Server (GHES). Given that the workflow file specifiesubuntu-latest
andubuntu-20.04
as the matrix platforms, and no specific compatibility issues were identified for these platforms, it appears that the action should work as expected in the specified environment.
- Matrix platforms specified in
build-db.yml
areubuntu-latest
andubuntu-20.04
.- No platform-specific compatibility issues were found in the action's documentation or source code for these platforms.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify compatibility of `actions/upload-artifact@v4` with all matrix platforms. # Test: Search for compatibility notes in the official GitHub action documentation. curl -s https://github.com/actions/upload-artifact/tree/v4 | grep -i "compatibility"Length of output: 84
Script:
#!/bin/bash # Description: Verify compatibility of `actions/upload-artifact@v4` with all matrix platforms. # Check the official documentation for compatibility details. curl -s https://github.com/actions/upload-artifact | grep -i "compatibility" # Review the release notes for platform compatibility information. curl -s https://github.com/actions/upload-artifact/releases | grep -i "compatibility" # Check issues or discussions in the GitHub repository for any known compatibility concerns. curl -s https://github.com/actions/upload-artifact/issues | grep -i "compatibility"Length of output: 246
Script:
#!/bin/bash # Description: Verify compatibility of `actions/upload-artifact@v4` with all matrix platforms. # Step 1: Examine the build-db.yml file to understand the matrix platforms specified. echo "Matrix platforms specified in build-db.yml:" grep -E 'matrix:|os:|runs-on:' .github/workflows/build-db.yml # Step 2: Search for any compatibility mentions within the action's repository using more specific queries. echo "Searching for compatibility mentions in the action's repository:" curl -s https://github.com/actions/upload-artifact | grep -iE "compatibility|platform|support" # Step 3: Check for any usage examples or documentation within the repository that might indicate platform compatibility. echo "Checking for usage examples or documentation:" curl -s https://github.com/actions/upload-artifact/tree/v4 | grep -i "example" # Step 4: Review the action's source code to look for any platform-specific code or comments. echo "Reviewing the action's source code for platform-specific code or comments:" curl -s https://raw.githubusercontent.com/actions/upload-artifact/v4/action.yml | grep -iE "platform|os|windows|linux|macos"Length of output: 182985
86-86
: Verify that theactions/download-artifact@v4
correctly handles the artifacts for thesdist
platform, especially since the upload is run twice whenmatrix.platform == 'sdist'
.Verification successful
The
build-db.yml
workflow file includes steps to handle artifacts for thesdist
platform usingactions/download-artifact@v4
. Thesdist
platform is built and uploaded as an artifact, and this artifact is downloaded in both thetest
andpublish
jobs. The artifact handling appears to be configured correctly.
- The
build
job has a step to upload artifacts with the namedist-${{ matrix.platform }}
.- The
test
job downloads thedist-sdist
artifact for testing.- The
publish
job downloads all artifacts for publishing.No issues were found with the artifact handling for the
sdist
platform.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify handling of artifacts for `sdist` platform with `actions/download-artifact@v4`. # Test: Check for special conditions or configurations needed for `sdist` in the action's documentation. curl -s https://github.com/actions/download-artifact/tree/v4 | grep -i "sdist"Length of output: 78
Script:
#!/bin/bash # Description: Check the `build-db.yml` workflow for any `sdist` related configurations or conditions. # Search for `sdist` in the `build-db.yml` file to identify relevant configurations. fd build-db.yml | xargs grep -i 'sdist'Length of output: 39
Script:
#!/bin/bash # Description: Output the contents of the `build-db.yml` file to understand the context and usage of `actions/download-artifact@v4`. # Display the contents of the `build-db.yml` file. fd build-db.yml | xargs catLength of output: 29
Script:
#!/bin/bash # Description: List all files in the repository to confirm the existence and path of the `build-db.yml` file. # List all files in the repository. fdLength of output: 131414
Script:
#!/bin/bash # Description: List all `.yml` files in the `.github/workflows/` directory to confirm the absence of `build-db.yml` and identify any other relevant workflow files. # List all `.yml` files in the `.github/workflows/` directory. fd .yml .github/workflows/Length of output: 366
Script:
#!/bin/bash # Description: Output the contents of the `build-db.yml` file to understand the context and usage of `actions/download-artifact@v4`. # Display the contents of the `build-db.yml` file. cat .github/workflows/build-db.ymlLength of output: 3669
I will have to manually merge this into teams; I'd like to get a squash commit before I do that to make the cherry-pick easier. On the teams side I'll |
The only failed test here is the PR-creation, which I have done manually here to get around a bone-headed move I made last September. |
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.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/build-db.yml (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/build-db.yml
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.
LGTM! 🙇
What changes are proposed in this pull request?
Bumps actions/download-artifact from 3 to 4.
Bumps actions-upload-artifact from 3 to 4.
the release notes identify that
upload
anddownload
need to be the same [major] version.Also, looks like we're running the upload twice when
matrix.platform == 'sdist'
which is a v4 no-noupdated-dependencies:
How is this patch tested? If it is not, please explain why.
PR tests below
Release Notes
Is this a user-facing change that should be mentioned in the release notes?
notes for FiftyOne users.
(Details in 1-2 sentences. You can just refer to another PR with a description
if this PR is part of a larger change.)
What areas of FiftyOne does this PR affect?
fiftyone
Python library changesSummary by CodeRabbit
download-artifact
version fromv3
tov4
across multiple workflow files.build-db
workflow.