Skip to content
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.

Sign up for GitHub

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

Adds unique name for upload-artifact jobs #2285

Merged
merged 6 commits into from
Jan 15, 2025
Merged

Adds unique name for upload-artifact jobs #2285

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1446eee
Adds unique name for upload-artifact jobs
gadial Jan 14, 2025
fe9e1bd
Attempt to install openblas in the neko run
gadial Jan 14, 2025
3273497
deploy names update and neko fix
gadial Jan 14, 2025
2e5a233
blas and lapack fix in the tutorials job as well
gadial Jan 14, 2025
def4f4f
Move the blas install earlier
gadial Jan 14, 2025
d9d7fb7
Bump all upload-artifact to v4 and distinguish between "shared" and "…
gadial Jan 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions .github/workflows/deploy.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ jobs:
- uses: actions/upload-artifact@v4
with:
path: ./wheelhouse/*.whl
name: publish-shared-wheels
name: aer-deploy-wheel-build-${{ matrix.os }}
build_wheels_aarch64:
name: Build wheels on ${{ matrix.os }}
runs-on: ${{ matrix.os }}
Expand Down Expand Up @@ -60,6 +60,7 @@ jobs:
- 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
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree. I changed this to "separate" in d9d7fb7 and set the upload prefix to handle "shared".

with:
Expand Down Expand Up @@ -87,7 +88,7 @@ jobs:
- uses: actions/upload-artifact@v3
with:
path: ./wheelhouse/*.whl
name: publish-shared-wheels
name: aer-deploy-wheel-arm64-macos-${{ matrix.os }}
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

sdist:
name: Publish qiskit-aer sdist
runs-on: ubuntu-latest
Expand All @@ -110,6 +111,7 @@ jobs:
- 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
Copy link
Member

Choose a reason for hiding this comment

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

Deployed right after - maybe give a aer-deploy-separate- prefix to avoid being pulled by the shared uploader?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added the prefix as you suggested in d9d7fb7

with:
Expand Down Expand Up @@ -154,6 +156,7 @@ jobs:
- 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
Copy link
Member

Choose a reason for hiding this comment

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

Deployed here. Also, it's still upload-artifact@v3.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Bumped to v4 and added "separate" in d9d7fb7

with:
Expand Down Expand Up @@ -198,6 +201,7 @@ jobs:
- 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
Copy link
Member

Choose a reason for hiding this comment

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

Deployed here. Also, it's upload-artifact@v3.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Bumped to 4 and added "separate" in d9d7fb7

with:
Expand Down Expand Up @@ -234,6 +238,7 @@ jobs:
- uses: actions/upload-artifact@v4
with:
path: ./wheelhouse/*.whl
name: aer-deploy-build_wheels_s390x-${{ matrix.os }}
- name: Publish package distributions to PyPI
uses: pypa/gh-action-pypi-publish@release/v1
with:
Expand Down Expand Up @@ -270,6 +275,7 @@ jobs:
- uses: actions/upload-artifact@v4
with:
path: ./wheelhouse/*.whl
name: aer-deploy-build_wheels_ppc64le-${{ matrix.os }}
- name: Publish package distributions to PyPI
uses: pypa/gh-action-pypi-publish@release/v1
with:
Expand All @@ -285,7 +291,8 @@ jobs:
- uses: actions/download-artifact@v4
with:
path: deploy
name: publish-shared-wheels
pattern: 'aer-deploy-'
merge-multiple: true
- name: Publish package distributions to PyPI
uses: pypa/gh-action-pypi-publish@release/v1
with:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/docs.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,12 @@ jobs:
- name: Setup tutorials job
run: |
set -e
sudo apt install -y graphviz pandoc libblas-dev liblapack-dev
git clone https://github.com/Qiskit/qiskit-tutorials --depth=1
python -m pip install --upgrade pip wheel
pip install -U -r requirements-dev.txt -c constraints.txt
pip install -c constraints.txt git+https://github.com/Qiskit/qiskit
pip install -c constraints.txt .
sudo apt install -y graphviz pandoc libopenblas-dev
pip check
shell: bash
- name: Run Tutorials
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/neko.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ jobs:
with:
test_selection: backend
repo_install_command: |
sudo apt-get install -y libblas-dev liblapack-dev
pip install scipy==1.13.1
pip install conan
pip uninstall -y qiskit qiskit-terra qiskit-aer
Expand Down
Loading