-
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 21 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,117 @@ | ||||||||||||||||
name: Publish xrpl-py 🐍 distribution 📦 to PyPI | ||||||||||||||||
on: | ||||||||||||||||
push: | ||||||||||||||||
tags: | ||||||||||||||||
- 'v*' # Run on version tags only | ||||||||||||||||
|
||||||||||||||||
jobs: | ||||||||||||||||
build: | ||||||||||||||||
name: Build distribution 📦 | ||||||||||||||||
runs-on: ubuntu-latest | ||||||||||||||||
env: | ||||||||||||||||
POETRY_VERSION: 1.8.3 | ||||||||||||||||
|
||||||||||||||||
steps: | ||||||||||||||||
- uses: actions/checkout@v4 | ||||||||||||||||
- name: Set up Python | ||||||||||||||||
uses: actions/setup-python@v5 | ||||||||||||||||
with: | ||||||||||||||||
python-version: ">=3.8,<3.13" | ||||||||||||||||
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. Don’t we have a file to specify the version of Python? 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. I haven't come across such a file. We specify all the supported versions of python explicitly in a list, here is an example:
But its a good idea to refactor these common elements from all the Github Actions workflows. I need to investigate how to link external files/ENV-VARIABLES inside these workflows. |
||||||||||||||||
- name: Load cached .local | ||||||||||||||||
id: cache-poetry | ||||||||||||||||
uses: actions/cache@v3 | ||||||||||||||||
with: | ||||||||||||||||
path: /home/runner/.local | ||||||||||||||||
key: dotlocal-${{ env.POETRY_VERSION }} | ||||||||||||||||
ckeshava marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||
- name: Install poetry | ||||||||||||||||
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 Enhance cache key specificity. The cache key only includes the Poetry version. Consider adding the runner OS and Python version for better isolation. - key: dotlocal-${{ env.POETRY_VERSION }}
+ key: dotlocal-${{ runner.os }}-py${{ matrix.python-version }}-poetry${{ env.POETRY_VERSION }}
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. @ckeshava agree here 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.
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. I'm not sure if this suggestion is helpful:
Having said that, if the cache is used across multiple branches, it is useful to include environment info in the key. I'm surprised we haven't hit any snags although we've used a "broad" key in the tests workflows. 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. I mean the concept of using more specific keys in general |
||||||||||||||||
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: Verify build artifacts | ||||||||||||||||
run: | | ||||||||||||||||
ls dist/*.whl dist/*.tar.gz || exit 1 | ||||||||||||||||
pip install check-wheel-contents | ||||||||||||||||
check-wheel-contents dist/*.whl | ||||||||||||||||
- name: Store the distribution packages | ||||||||||||||||
uses: actions/upload-artifact@v4 | ||||||||||||||||
with: | ||||||||||||||||
name: python-package-distributions | ||||||||||||||||
path: dist/ | ||||||||||||||||
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
|
||||||||||||||||
verbose: true | ||||||||||||||||
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. Remove duplicate 'verbose' key. The 'verbose' key is duplicated in the PyPI publish step configuration. with:
verbose: true
- verbose: true 📝 Committable suggestion
Suggested change
🧰 Tools🪛 actionlint66-66: key "verbose" is duplicated in "with" section. previously defined at line:65,col:9. note that this key is case insensitive (syntax-check) 🪛 yamllint[error] 66-66: duplication of key "verbose" in mapping (key-duplicates) |
||||||||||||||||
|
||||||||||||||||
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 }} | ||||||||||||||||
# 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 using the `gh` CLI. | ||||||||||||||||
# `dist/` contains the built packages, and the | ||||||||||||||||
# sigstore-produced signatures and certificates. | ||||||||||||||||
gh release upload | ||||||||||||||||
'${{ github.ref_name }}' dist/** | ||||||||||||||||
--repo '${{ github.repository }}' | ||||||||||||||||
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. Fix release upload command formatting. The current command formatting could cause issues. Each argument should be on a new line with proper continuation. - gh release upload
- '${{ github.ref_name }}' dist/**
- --repo '${{ github.repository }}'
+ 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.
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 comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I have not explicitly documented the usage of
poetry
. At the moment, it is used in all the Github Actions workflows.In another PR, the version of poetry has already been updated to
1.8.3
in all the Github Actions workflows. I need to rebase this feature branch.Are you referring something else for documentation?