-
Notifications
You must be signed in to change notification settings - Fork 87
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
Use Github-Actions for PyPI release process #760
Changes from all commits
a6ffae5
c63b7e4
20a1d9e
a7d2fee
f99a483
7fe66c5
39e344c
e81ac65
80d1061
5d32fae
441725f
3d4553e
4c81e46
cf22e32
51018bf
bc2d821
9bcda8e
64cf7da
cde467c
c9ee19a
71f529e
817d97d
fd487b7
e0260b4
1c77cfb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,107 @@ | ||||||||||||||||||||||||||||||||||
name: Publish xrpl-py 🐍 distribution 📦 to PyPI | ||||||||||||||||||||||||||||||||||
on: | ||||||||||||||||||||||||||||||||||
push: | ||||||||||||||||||||||||||||||||||
tags: | ||||||||||||||||||||||||||||||||||
- '*' | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
jobs: | ||||||||||||||||||||||||||||||||||
build: | ||||||||||||||||||||||||||||||||||
name: Build distribution 📦 | ||||||||||||||||||||||||||||||||||
runs-on: ubuntu-latest | ||||||||||||||||||||||||||||||||||
env: | ||||||||||||||||||||||||||||||||||
POETRY_VERSION: 1.8.3 | ||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you document this and other places where we use poetry to give contributors and understanding where all they need to make changes There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, I have not explicitly documented the usage of In another PR, the version of poetry has already been updated to Are you referring something else for documentation? |
||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
steps: | ||||||||||||||||||||||||||||||||||
- uses: actions/checkout@v4 | ||||||||||||||||||||||||||||||||||
- name: Set up Python | ||||||||||||||||||||||||||||||||||
uses: actions/setup-python@v5 | ||||||||||||||||||||||||||||||||||
with: | ||||||||||||||||||||||||||||||||||
# Use the lowest supported version of Python for CI/CD | ||||||||||||||||||||||||||||||||||
python-version: "3.8" | ||||||||||||||||||||||||||||||||||
- name: Load cached .local | ||||||||||||||||||||||||||||||||||
id: cache-poetry | ||||||||||||||||||||||||||||||||||
uses: actions/cache@v3 | ||||||||||||||||||||||||||||||||||
with: | ||||||||||||||||||||||||||||||||||
path: /home/runner/.local | ||||||||||||||||||||||||||||||||||
key: dotlocal-${{ env.POETRY_VERSION }}-${{ hashFiles('poetry.lock') }} | ||||||||||||||||||||||||||||||||||
- name: Install poetry | ||||||||||||||||||||||||||||||||||
if: steps.cache-poetry.outputs.cache-hit != 'true' | ||||||||||||||||||||||||||||||||||
run: | | ||||||||||||||||||||||||||||||||||
curl -sSL "https://install.python-poetry.org/" | python - --version "${{ env.POETRY_VERSION }}" | ||||||||||||||||||||||||||||||||||
echo "${HOME}/.local/bin" >> $GITHUB_PATH | ||||||||||||||||||||||||||||||||||
poetry --version || exit 1 # Verify installation | ||||||||||||||||||||||||||||||||||
- name: Build a binary wheel and a source tarball | ||||||||||||||||||||||||||||||||||
run: poetry build | ||||||||||||||||||||||||||||||||||
Comment on lines
+33
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add build artifact verification step. Add verification of built artifacts before uploading to ensure the build process completed successfully. - name: Build a binary wheel and a source tarball
run: poetry build
+ - name: Verify built artifacts
+ run: |
+ ls dist/*.whl dist/*.tar.gz || exit 1 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||
- name: Store the distribution packages | ||||||||||||||||||||||||||||||||||
uses: actions/upload-artifact@v4 | ||||||||||||||||||||||||||||||||||
with: | ||||||||||||||||||||||||||||||||||
name: python-package-distributions | ||||||||||||||||||||||||||||||||||
path: dist/ | ||||||||||||||||||||||||||||||||||
ckeshava marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||
publish-to-pypi: | ||||||||||||||||||||||||||||||||||
name: >- | ||||||||||||||||||||||||||||||||||
Publish Python 🐍 distribution 📦 to PyPI | ||||||||||||||||||||||||||||||||||
needs: build # Explicit dependency on build job | ||||||||||||||||||||||||||||||||||
runs-on: ubuntu-latest | ||||||||||||||||||||||||||||||||||
timeout-minutes: 10 # Adjust based on typical publishing time | ||||||||||||||||||||||||||||||||||
permissions: | ||||||||||||||||||||||||||||||||||
# More information about Trusted Publishing and OpenID Connect: https://blog.pypi.org/posts/2023-04-20-introducing-trusted-publishers/ | ||||||||||||||||||||||||||||||||||
id-token: write # IMPORTANT: mandatory for trusted publishing | ||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Link to docs There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are you referring to Github Actions docs? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When you say something is mandatory, it’s best to link to why There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in 71f529e |
||||||||||||||||||||||||||||||||||
steps: | ||||||||||||||||||||||||||||||||||
- name: Download all the dists | ||||||||||||||||||||||||||||||||||
uses: actions/download-artifact@v4 | ||||||||||||||||||||||||||||||||||
with: | ||||||||||||||||||||||||||||||||||
name: python-package-distributions | ||||||||||||||||||||||||||||||||||
path: dist/ | ||||||||||||||||||||||||||||||||||
- name: Verify downloaded artifacts | ||||||||||||||||||||||||||||||||||
run: | | ||||||||||||||||||||||||||||||||||
ls dist/*.whl dist/*.tar.gz || exit 1 | ||||||||||||||||||||||||||||||||||
- name: Publish distribution 📦 to PyPI | ||||||||||||||||||||||||||||||||||
uses: pypa/gh-action-pypi-publish@release/v1 | ||||||||||||||||||||||||||||||||||
with: | ||||||||||||||||||||||||||||||||||
verbose: true | ||||||||||||||||||||||||||||||||||
ckeshava marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||
verify-metadata: true | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
github-release: | ||||||||||||||||||||||||||||||||||
name: >- | ||||||||||||||||||||||||||||||||||
Sign the Python 🐍 distribution 📦 with Sigstore | ||||||||||||||||||||||||||||||||||
and upload them to GitHub Release | ||||||||||||||||||||||||||||||||||
needs: | ||||||||||||||||||||||||||||||||||
- publish-to-pypi | ||||||||||||||||||||||||||||||||||
runs-on: ubuntu-latest | ||||||||||||||||||||||||||||||||||
timeout-minutes: 15 # Adjust based on typical signing and release time | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
permissions: | ||||||||||||||||||||||||||||||||||
contents: write # IMPORTANT: mandatory for making GitHub Releases | ||||||||||||||||||||||||||||||||||
id-token: write # IMPORTANT: mandatory for sigstore | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
steps: | ||||||||||||||||||||||||||||||||||
- name: Download all the dists | ||||||||||||||||||||||||||||||||||
uses: actions/download-artifact@v4 | ||||||||||||||||||||||||||||||||||
with: | ||||||||||||||||||||||||||||||||||
name: python-package-distributions | ||||||||||||||||||||||||||||||||||
path: dist/ | ||||||||||||||||||||||||||||||||||
- name: Sign the dists with Sigstore | ||||||||||||||||||||||||||||||||||
uses: sigstore/[email protected] | ||||||||||||||||||||||||||||||||||
with: | ||||||||||||||||||||||||||||||||||
inputs: >- | ||||||||||||||||||||||||||||||||||
./dist/*.tar.gz | ||||||||||||||||||||||||||||||||||
./dist/*.whl | ||||||||||||||||||||||||||||||||||
- name: Create GitHub Release | ||||||||||||||||||||||||||||||||||
env: | ||||||||||||||||||||||||||||||||||
GITHUB_TOKEN: ${{ github.token }} | ||||||||||||||||||||||||||||||||||
run: >- | ||||||||||||||||||||||||||||||||||
gh release create | ||||||||||||||||||||||||||||||||||
'${{ github.ref_name }}' | ||||||||||||||||||||||||||||||||||
--repo '${{ github.repository }}' | ||||||||||||||||||||||||||||||||||
--generate-notes || | ||||||||||||||||||||||||||||||||||
(echo "::error::Failed to create release" && exit 1) | ||||||||||||||||||||||||||||||||||
- name: Upload artifact signatures to GitHub Release | ||||||||||||||||||||||||||||||||||
env: | ||||||||||||||||||||||||||||||||||
GITHUB_TOKEN: ${{ github.token }} | ||||||||||||||||||||||||||||||||||
# Upload to GitHub Release using the `gh` CLI. | ||||||||||||||||||||||||||||||||||
# `dist/` contains the built packages, and the | ||||||||||||||||||||||||||||||||||
# sigstore-produced signatures and certificates. | ||||||||||||||||||||||||||||||||||
run: >- | ||||||||||||||||||||||||||||||||||
gh release upload | ||||||||||||||||||||||||||||||||||
'${{ github.ref_name }}' dist/** | ||||||||||||||||||||||||||||||||||
--repo '${{ github.repository }}' | ||||||||||||||||||||||||||||||||||
Comment on lines
+104
to
+107
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve release upload command formatting and verification. Add signature file verification and improve command readability. + # Verify signature files exist
+ run: |
+ if ! ls dist/*.sigstore >/dev/null 2>&1; then
+ echo "::error::Signature files not found"
+ exit 1
+ fi
# Upload to GitHub Release
- run: >-
- gh release upload
- '${{ github.ref_name }}' dist/**
- --repo '${{ github.repository }}'
+ run: |
+ gh release upload \
+ '${{ github.ref_name }}' \
+ dist/** \
+ --repo '${{ github.repository }}' 📝 Committable suggestion
Suggested change
|
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.
Restrict tag pattern to version tags.
The current pattern
'*'
will trigger on any tag. Since this is a release workflow, it should only trigger on version tags.📝 Committable suggestion
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.
Since there is a possibility of releasing specific feature-builds (like AMM, MPT, etc), I will not incorporate this suggestion. I prefer to leave some flexibility in definition of the tags.
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.
@ckeshava, understood. I'll note this preference for future reviews.
(_/)
( ._.)
/
✏️ Learnings added