Adds unique name for upload-artifact jobs #2285
Conversation
There was a problem hiding this comment.
You'll need to make sure that the corresponding download-wheels action is also updated, and likely use a prefix that you can reliably know, to glob against. I think github.job will be different for each of these runs, so you might struggle to write a usable glob.
(e.g. name: dist-${{ 'whatever' }}-${{ 'whatever' }} in all uploads, then
- uses: actions/download-artifact@v4
with:
pattern: 'dist-*'
merge-multiple: true
There was a problem hiding this comment.
The top comment explains a little more, but a final comment (from my perspective): technically, it's possible to set
with:
skip-existing: truein the config of the PyPI upload step, which would mean that the race condition on the separate wheels accidentally getting pulled into the shared wheels upload wouldn't matter - anything already uploaded would just be silently skipped. That's the historic behaviour of twine.
That said, it's probably best just to fix the prefixes to not need to bother - it's easy enough for us to just avoid the race condition entirely, and if we ever need to retrigger the job to rebuild, we can just modify it on-the-fly (since we'd have to be modifying the script anyway).
.github/workflows/deploy.yml
Outdated
| - uses: actions/upload-artifact@v4 | ||
| with: | ||
| path: ./wheelhouse/*.whl | ||
| name: aer-deploy-build_wheels_aarch64-${{ matrix.os }} | ||
| - name: Publish package distributions to PyPI | ||
| uses: pypa/gh-action-pypi-publish@release/v1 |
There was a problem hiding this comment.
Strictly, this set of artifacts (and a couple more noted below) gets uploaded in the line right below this one, so it doesn't need to be pulled down by the download-artifact run in the publish_shared_wheels job. Perhaps we could have the prefix be deploy-shared-, then jobs that previously uploaded to the publish-shared-wheels artifact name can become (e.g.) deploy-shared-x86_64-${{ matrix.os }} and deploy-shared-arm64-macos, while the others become deploy-separate-aarch64-linux etc? So then the glob is on deploy-shared-*, which won't attempt to re-download the wheels that aren't marked as job dependencies.
There was a problem hiding this comment.
I agree. I changed this to "separate" in d9d7fb7 and set the upload prefix to handle "shared".
.github/workflows/deploy.yml
Outdated
| - uses: actions/upload-artifact@v3 | ||
| with: | ||
| path: ./wheelhouse/*.whl | ||
| name: publish-shared-wheels | ||
| name: aer-deploy-wheel-arm64-macos-${{ matrix.os }} |
There was a problem hiding this comment.
This is still upload-artifact@v3 and needs bumping or the download-artifact@v4 won't be able to find it.
This is the only other upload-artifact run (along with the top comment) that gets downloaded by the shared uploader - all the others below it in this file might not want to match the prefix exactly.
There was a problem hiding this comment.
Bumped to v4 in d9d7fb7 and changed the name to include "shared". I was hesitant to change to v4 in my original PR since I am not aware of the reasons to the original bump to v4 and why only it affected only part of the file.
There was a problem hiding this comment.
The change from v3 to v4 was a huge breaking change in the upload/download-artifact jobs. You certainly always have to match (v4 download can't find v3 uploads, and so on), and I think GitHub will be pushing to deprecate and remove the old v3 system soon too - iirc, it was completely piggy-backed onto their logging framework, and never properly designed as an artifact store.
.github/workflows/deploy.yml
Outdated
| - uses: actions/upload-artifact@v4 | ||
| with: | ||
| path: ./dist/qiskit* | ||
| name: aer-deploy-sdist-${{ matrix.os }} | ||
| - name: Publish package distributions to PyPI | ||
| uses: pypa/gh-action-pypi-publish@release/v1 |
There was a problem hiding this comment.
Deployed right after - maybe give a aer-deploy-separate- prefix to avoid being pulled by the shared uploader?
There was a problem hiding this comment.
Added the prefix as you suggested in d9d7fb7
.github/workflows/deploy.yml
Outdated
| - uses: actions/upload-artifact@v3 | ||
| with: | ||
| path: ./wheelhouse/*.whl | ||
| name: aer-deploy-gpu-build-cuda11-${{ matrix.os }} | ||
| - name: Publish package distributions to PyPI | ||
| uses: pypa/gh-action-pypi-publish@release/v1 |
There was a problem hiding this comment.
Deployed here. Also, it's still upload-artifact@v3.
There was a problem hiding this comment.
Bumped to v4 and added "separate" in d9d7fb7
.github/workflows/deploy.yml
Outdated
| - uses: actions/upload-artifact@v3 | ||
| with: | ||
| path: ./wheelhouse/*.whl | ||
| name: aer-deploy-gpu-build-cuda12-${{ matrix.os }} | ||
| - name: Publish package distributions to PyPI | ||
| uses: pypa/gh-action-pypi-publish@release/v1 |
There was a problem hiding this comment.
Deployed here. Also, it's upload-artifact@v3.
There was a problem hiding this comment.
Bumped to 4 and added "separate" in d9d7fb7
…separete" uploads
Summary
This PR fixes an error in the github action for
deploypreventing wheels from being uploaded.Details and comments
When using
actions/upload-artifact@v4a unique name should be given for separate jobs, otherwise the name clash results inError: Failed to CreateArtifact: Received non-retryable error: Failed request: (409) Conflict: an artifact with this name already exists on the workflow run.This PR adds a unique name to the action.