DEVOPS-1154: [WIP] Devops 1154 - #216
Conversation
There was a problem hiding this comment.
Pull request overview
This PR appears to transition PyPI publishing from token-based uploads inside reusable workflows to an OIDC “trusted publishing” approach by introducing a composite action and removing the direct pypa/gh-action-pypi-publish steps from the reusable workflows.
Changes:
- Removed the “Publish package to PyPI” step from two reusable workflows that previously published to PyPI/TestPyPI.
- Added a new composite action (
reusable-python-publish_to_pypi) intended to publish distributions to PyPI/TestPyPI via trusted publishing, supporting artifact or release-asset sources.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
.github/workflows/reusable-python-release_pypi_assets.yml |
Removes the PyPI/TestPyPI publish step from the release-asset publishing workflow. |
.github/workflows/reusable-python-publish_pypi_package.yml |
Removes the PyPI/TestPyPI publish step from the package publish workflow. |
.github/actions/reusable-python-publish_to_pypi/action.yml |
Adds a composite action to publish to PyPI/TestPyPI using OIDC trusted publishing. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| artifactory-dir-path: ${{ matrix.virtual-repo-name }}/${{ inputs.package-name }}/${{ inputs.release-tag }} | ||
| JFROG_ARTIFACTORY_URL: ${{ secrets.JFROG_ARTIFACTORY_URL }} | ||
| JFROG_ARTIFACTORY_TOKEN: ${{ secrets.JFROG_ARTIFACTORY_TOKEN }} |
There was a problem hiding this comment.
suggestion: report error on attempts to publish to pypi / test-pypi
as suggested by Copilot
| Must be invoked as a step in a job defined directly in the caller's own | ||
| top-level workflow file (not from within a reusable *workflow*), so the | ||
| OIDC token's workflow_ref matches the repository's configured PyPI | ||
| trusted publisher. |
| artifactory-dir-path: ${{ matrix.virtual-repo-name }}/${{ inputs.package-name }}/${{ env.version }} | ||
| JFROG_ARTIFACTORY_URL: ${{ secrets.JFROG_ARTIFACTORY_URL }} | ||
| JFROG_ARTIFACTORY_TOKEN: ${{ secrets.JFROG_ARTIFACTORY_TOKEN }} |
There was a problem hiding this comment.
suggestion: report error on attempts to publish to pypi / test-pypi
as suggested by Copilot
| - name: Download build artifact | ||
| if: ${{ inputs.source == 'artifact' }} | ||
| uses: actions/download-artifact@v8 | ||
| with: | ||
| name: ${{ inputs.artifact-name }} | ||
| path: dist-download | ||
| - name: Download release assets |
504ca60 to
3feab80
Compare
|
|
sebhmg
left a comment
sebhmg
left a comment
There was a problem hiding this comment.
suggestion: have 2 different simpler actions instead of bundling 2 uses-case into one action
| artifactory-dir-path: ${{ matrix.virtual-repo-name }}/${{ inputs.package-name }}/${{ inputs.release-tag }} | ||
| JFROG_ARTIFACTORY_URL: ${{ secrets.JFROG_ARTIFACTORY_URL }} | ||
| JFROG_ARTIFACTORY_TOKEN: ${{ secrets.JFROG_ARTIFACTORY_TOKEN }} |
There was a problem hiding this comment.
suggestion: report error on attempts to publish to pypi / test-pypi
as suggested by Copilot
| artifactory-dir-path: ${{ matrix.virtual-repo-name }}/${{ inputs.package-name }}/${{ env.version }} | ||
| JFROG_ARTIFACTORY_URL: ${{ secrets.JFROG_ARTIFACTORY_URL }} | ||
| JFROG_ARTIFACTORY_TOKEN: ${{ secrets.JFROG_ARTIFACTORY_TOKEN }} |
There was a problem hiding this comment.
suggestion: report error on attempts to publish to pypi / test-pypi
as suggested by Copilot
| source: | ||
| description: 'Where to fetch the built packages from: "artifact" or "release"' | ||
| required: true | ||
| type: string | ||
| artifact-name: | ||
| description: 'Name of the workflow artifact to download. Required when source is "artifact"' | ||
| required: false | ||
| type: string | ||
| default: '' | ||
| release-tag: | ||
| description: 'Tag of the GitHub release to download assets from. Required when source is "release"' | ||
| required: false | ||
| type: string | ||
| default: '' | ||
| github-token: | ||
| description: 'Token used to download release assets via "gh release download". Required when source is "release"' | ||
| required: false | ||
| type: string | ||
| default: '' |
There was a problem hiding this comment.
suggestion: keep tow different actions: one for release, one for build artifact
within this single action, there is extra complexity with the requirement on expect params depending on value of source, and parsm being exclusive
As use case as separeted, having 2 different actions seem to make sense: publishing to prod will always fetch from release, while publishing to dev will always fetch from build.
DEVOPS-1154 - address warnings upon publishing Python packages from GitHub
testing with MiraGeoscience/simpeg#166