-
Notifications
You must be signed in to change notification settings - Fork 19
update publishing flow to use ni/python-actions #227
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
update publishing flow to use ni/python-actions #227
Conversation
|
Thank you for contributing! 👋 |
|
@bkeryan, does this look right? (also, not sure how to test before merging...) |
After merging, it will show up in the UI, but, to test before merging:
|
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.
See https://github.com/ni/nitypes-python/blob/main/.github/workflows/publish.yml for a working example
| - name: Upload Python package | ||
| if: ${{ github.event.release.target_commitish == 'main' || startsWith(github.event.release.target_commitish, 'releases/')}} | ||
| uses: pypa/gh-action-pypi-publish@76f52bc884231f62b9a034ebfe128415bbaabdfc # v1.12.4 |
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.
Trusted publishing will not work without specifying the "pypi" environment and "id-token: write" permissions. See https://github.com/ni/nitypes-python/blob/main/.github/workflows/publish.yml#L66
Also, it is not secure to run poetry install, pytest, etc. in the "pypi" environment. You should split the workflow into multiple jobs:
- build/test, upload artifact (default permissions)
- download artifact, upload to pypi ("pypi" environment, "id-token: write" permissions)
- update project version (default permissions)
Also, you can test the publish workflow with TestPyPI by creating an account on test.pypi.org, editing the workflow to use testpypi by default, and publishing a release in your personal fork of the repo. |
github: Get Trusted Publisher workflow working
| on: | ||
| release: | ||
| types: [released] | ||
| types: [published] |
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.
FYI, published is equivalent to [released, prereleased], so it allows you to publish prereleases. However, this repo's versioning strategy does not currently support this.
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.
From what I was reading, they're also different in that
"released" is triggered when a release (non-draft) is made
"published" is truggered when a non-draft release is or a draft release is marked as non-draft.
(unless I read that wrong...)
If that is the case, this is a favorable change despite the version limitations
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.
The note on https://docs.github.com/en/actions/reference/events-that-trigger-workflows#release makes it sound like "released" will work with "Save draft" + "Publish release" but "prereleased" will not work with "[x] Set as a pre-release" + "Save draft" + "Publish release". I think we may have run into this when we started publishing measurement-plugin-python pre-releases.
| # If the version is 0.1.0-alpha.0, this will set the version to 0.1.0 | ||
| - name: Promote package version to release | ||
| run: poetry version patch |
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.
FYI, we are not doing this in other Python projects:
- Modifying the project version before publishing causes the tagged commit and the release .zip file to have the wrong version. This has been an annoyance for the measurement-plugin-python project.
- Converting alpha/beta/dev versions to release before publishing means you can't publish alpha/beta/dev versions to PyPI.
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.
On point b, we (PWG) decided to do this so that "alpha" always means "from source", not saying that was a good decision, just what this was setup with.
* update publishing flow to use ni/python-actions * github: Split Publish-Package.yml jobs according to security best practices * github: Restore versioning policy * github: Fix next version update --------- Co-authored-by: Brad Keryan <brad.keryan@ni.com>
Publishing is currently broken due to using an old PAT.
As part of switching to OIDC based publishing, we should bring in the update ni/python-actions templates