Skip to content
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

Merged
merged 25 commits into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
a6ffae5
Initial draft of GIthub Actions workflow
ckeshava Oct 24, 2024
c63b7e4
install (or) load poetry from cache
ckeshava Oct 24, 2024
20a1d9e
Github Actions: Publish tagged commits into PyPI
ckeshava Oct 24, 2024
a7d2fee
Update version on the least-significant-digit -- this should not trig…
ckeshava Oct 24, 2024
f99a483
Update minor version of the project
ckeshava Oct 24, 2024
7fe66c5
Update the instructions for the Release process
ckeshava Oct 25, 2024
39e344c
Include sigstore signatures, automatic github-release process
ckeshava Oct 26, 2024
e81ac65
auto-generate release notes
ckeshava Oct 26, 2024
80d1061
revert the changes to project name and project-version
ckeshava Oct 28, 2024
5d32fae
include code-rabbit's suggestions
ckeshava Oct 28, 2024
441725f
Update .github/workflows/publish_to_pypi.yml
ckeshava Oct 28, 2024
3d4553e
Update .github/workflows/publish_to_pypi.yml
ckeshava Oct 28, 2024
4c81e46
Update .github/workflows/publish_to_pypi.yml
ckeshava Oct 28, 2024
cf22e32
Update .github/workflows/publish_to_pypi.yml
ckeshava Oct 28, 2024
51018bf
incoroprate CodeRabbit suggestions: part 2
ckeshava Oct 28, 2024
bc2d821
CodeRabbitAI suggestions part-3
ckeshava Oct 28, 2024
9bcda8e
remove the redundant check condition
ckeshava Oct 28, 2024
64cf7da
Merge branch 'main' into ga-pypi-release
justinr1234 Oct 29, 2024
cde467c
Update .github/workflows/publish_to_pypi.yml
ckeshava Oct 29, 2024
c9ee19a
Update .github/workflows/publish_to_pypi.yml
ckeshava Oct 29, 2024
71f529e
include resources about Trusted Publishing on PyPI
ckeshava Oct 29, 2024
817d97d
Rectify the errors in the Github Actions file
ckeshava Oct 29, 2024
fd487b7
Update .github/workflows/publish_to_pypi.yml
ckeshava Oct 29, 2024
e0260b4
Update .github/workflows/publish_to_pypi.yml
ckeshava Oct 29, 2024
1c77cfb
Merge branch 'main' into ga-pypi-release
ckeshava Nov 4, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions .github/workflows/publish_to_pypi.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
name: Publish xrpl-py 🐍 distribution 📦 to PyPI
env:
POETRY_VERSION: 1.8.3

on: push
justinr1234 marked this conversation as resolved.
Show resolved Hide resolved
jobs:
build:
name: Build distribution 📦
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v4
- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: "3.x"
justinr1234 marked this conversation as resolved.
Show resolved Hide resolved
- 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
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 24, 2024

Choose a reason for hiding this comment

The 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 }}

Committable suggestion was skipped due to low confidence.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ckeshava agree here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this suggestion is helpful:

  1. I'm using the default version of Python 3.x provided by this Github Actions runner. I believe this script uses only one version of Python.
- name: Set up Python
      uses: actions/setup-python@v5
      with:
        python-version: "3.x"
  1. This workflow is executed only on ubuntu-latest, not on any other operating systems.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The 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
ckeshava marked this conversation as resolved.
Show resolved Hide resolved
- name: Build a binary wheel and a source tarball
run: poetry build
Comment on lines +33 to +34
Copy link
Contributor

Choose a reason for hiding this comment

The 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

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Build a binary wheel and a source tarball
run: poetry build
- 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

- 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
if: startsWith(github.ref, 'refs/tags/') # only publish to PyPI on tag pushes
runs-on: ubuntu-latest
ckeshava marked this conversation as resolved.
Show resolved Hide resolved
permissions:
id-token: write # IMPORTANT: mandatory for trusted publishing
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to docs

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you referring to Github Actions docs?
id-token refers to the temporary OpenConnectID token that's assigned to Github Actions by PyPI. I've included docs inside the CONTRIBUTING.md file.

Copy link
Collaborator

Choose a reason for hiding this comment

The 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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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: Publish distribution 📦 to PyPI
uses: pypa/gh-action-pypi-publish@release/v1
justinr1234 marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 2 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tool.poetry]
name = "xrpl-py"
version = "3.0.0"
name = "xrpl-py-release-test"
justinr1234 marked this conversation as resolved.
Show resolved Hide resolved
version = "3.0.2"
description = "A complete Python library for interacting with the XRP ledger"
readme = "README.md"
repository = "https://github.com/XRPLF/xrpl-py"
Expand Down
Loading