Restore Python release CI workflow with artifact download fixes#77
Conversation
This restores the Python CI workflow that was previously removed due to broken artifact downloads in the `create-github-release` and `publish-to-pypi` jobs. The jobs were attempting to download artifacts using matrix variables that didn't exist in those jobs' scope. Fixed by using `pattern: dist-packages-*` to download all matching artifacts and `merge-multiple: true` to merge them into a single directory. Additionally, added `environment: release` to the `publish-to-pypi` job to enable PyPI trusted publishing. The workflow is now suitable for releasing Python packages to PyPI on tag creation. Signed-off-by: Phillip Wenig <phillip.wenig@frequenz.com>
There was a problem hiding this comment.
Pull request overview
This PR restores the Python CI workflow that was previously removed due to broken artifact downloads. The workflow implements a complete CI/CD pipeline for a Rust+Python hybrid project using maturin, including testing, building, documentation generation, GitHub releases, and PyPI publishing. The PR addresses artifact download issues by using pattern matching (pattern: dist-packages-*) with merge-multiple: true instead of undefined matrix variables, and adds trusted publishing support for PyPI via the environment: release configuration.
Changes:
- Added new Python CI workflow file (
.github/workflows/python-ci.yaml) with matrix-based testing and building across multiple platforms and Python versions - Fixed artifact download in release and PyPI publishing jobs using wildcard patterns with merge-multiple
- Added trusted publishing configuration for PyPI with appropriate permissions
- Updated RELEASE_NOTES.md to document the restoration of the Python release CI workflow
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
.github/workflows/python-ci.yaml |
New workflow file implementing complete CI/CD pipeline with nox testing, multi-platform builds, documentation publishing, GitHub releases, and PyPI publishing |
RELEASE_NOTES.md |
Added bug fix entry documenting the restoration of Python release CI workflow |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| with: | ||
| python-version: ${{ env.DEFAULT_PYTHON_VERSION }} | ||
| dependencies: build | ||
|
|
There was a problem hiding this comment.
The build job is missing Rust toolchain setup. This is a maturin-based project that builds Rust code with Python bindings. The python -m build command will invoke maturin, which requires Rust to be installed. Add a step to setup Rust (e.g., using actions-rust-lang/setup-rust-toolchain action) before the build step, or verify that the gh-action-setup-python-with-deps action handles Rust setup for maturin projects.
| - name: Setup Rust toolchain | |
| uses: actions-rust-lang/setup-rust-toolchain@v1 | |
| with: | |
| toolchain: stable |
There was a problem hiding this comment.
This is taken care of by maturin, right?
| - name: Setup Python | ||
| uses: frequenz-floss/gh-action-setup-python-with-deps@v1.0.1 | ||
| with: | ||
| python-version: ${{ env.DEFAULT_PYTHON_VERSION }} |
There was a problem hiding this comment.
The build job matrix includes multiple Python versions (3.11, 3.12, 3.13), but the setup step always uses the DEFAULT_PYTHON_VERSION (3.11). This means all builds will be performed with Python 3.11 regardless of the matrix.python value, making the Python version dimension of the matrix ineffective. Change line 117 to use python-version: ${{ matrix.python }} to build with the correct Python version from the matrix.
| python-version: ${{ env.DEFAULT_PYTHON_VERSION }} | |
| python-version: ${{ matrix.python }} |
| python-version: ${{ env.DEFAULT_PYTHON_VERSION }} | ||
| dependencies: build | ||
|
|
||
| - name: Build the source and binary distribution |
There was a problem hiding this comment.
The build job defines multiple platform targets (x86_64, x86, aarch64, armv7) in the matrix, but the build command python -m build does not use the matrix.platform.target variable. For cross-compilation with maturin, you need to specify the target architecture. Consider using maturin directly with the --target flag or configuring the build system to use the target from the matrix. Otherwise, all builds will target the host architecture only, and the target dimension of the matrix serves no purpose.
| - name: Build the source and binary distribution | |
| - name: Build the source and binary distribution | |
| env: | |
| MATURIN_TARGET: ${{ matrix.platform.target }} |
There was a problem hiding this comment.
I guess this is also handled by maturin by using the same target it is running on?
llucax
left a comment
llucax
left a comment
There was a problem hiding this comment.
I'm assuming the main contents here were used before and are already tested, so not looking into it very in depth. I wen't through copilot comments and resolved the ones that were clearly wrong. There is one comment that is a real bug and should be addressed.
|
|
||
| ## Bug Fixes | ||
|
|
||
| - Restored Python release CI workflow with fixes for artifact downloads. |
There was a problem hiding this comment.
Nitpick: I guess the most visible change here for users is the releases are being pushed to PyPI again, the rest I think is only internal project details.
| with: | ||
| python-version: ${{ env.DEFAULT_PYTHON_VERSION }} | ||
| dependencies: build | ||
|
|
There was a problem hiding this comment.
This is taken care of by maturin, right?
| python-version: ${{ env.DEFAULT_PYTHON_VERSION }} | ||
| dependencies: build | ||
|
|
||
| - name: Build the source and binary distribution |
There was a problem hiding this comment.
I guess this is also handled by maturin by using the same target it is running on?
Summary
create-github-releaseandpublish-to-pypijobs by usingpattern: dist-packages-*withmerge-multiple: trueinstead of undefined matrix variablesenvironment: releaseto enable PyPI trusted publishingCloses #70