Use Github-Actions for PyPI release process #760
Conversation
WalkthroughThis pull request introduces a new GitHub Actions workflow in Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Here is a sample package that's published by this Github Actions workflow: https://pypi.org/project/xrpl-py-release-test/#description Let me know if something is amiss. I verified that this package is published correctly, through |
|
I need to use some tags to test the publishing of "tagged commits". I will delete these test-tags once my testing is complete. |
…ger PyPI publish Github Actions
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (1)
.github/workflows/publish_to_pypi.yml (1)
1-49: Consider adding environment protection rules.For additional security, consider defining a protected environment for PyPI deployments. This allows you to:
- Require approval before publishing
- Restrict which branches can deploy
- Limit which users/teams can approve deployments
Add environment configuration:
publish-to-pypi: name: >- Publish Python 🐍 distribution 📦 to PyPI if: startsWith(github.ref, 'refs/tags/') runs-on: ubuntu-latest + environment: + name: pypi + url: https://pypi.org/project/xrpl-py/ permissions: id-token: writeThen configure environment protection rules in your repository settings.
🧰 Tools
🪛 actionlint
25-25: shellcheck reported issue in this script: SC2086:info:2:28: Double quote to prevent globbing and word splitting
(shellcheck)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- .github/workflows/publish_to_pypi.yml (1 hunks)
- pyproject.toml (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- pyproject.toml
🧰 Additional context used
🪛 actionlint
.github/workflows/publish_to_pypi.yml
25-25: shellcheck reported issue in this script: SC2086:info:2:28: Double quote to prevent globbing and word splitting
(shellcheck)
| - name: Load cached .local | ||
| id: cache-poetry | ||
| uses: actions/cache@v3 | ||
| with: | ||
| path: /home/runner/.local | ||
| key: dotlocal-${{ env.POETRY_VERSION }} | ||
| - name: Install poetry |
There was a problem hiding this comment.
🛠️ 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.
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
I'm not sure if this suggestion is helpful:
- 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"
- 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.
There was a problem hiding this comment.
I mean the concept of using more specific keys in general
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
CONTRIBUTING.md (2)
207-207: Add context about GPG signing for tag creation.The
-sflag in the tag command indicates GPG signing. Consider adding a note about GPG key setup requirements, as this might confuse contributors who haven't configured GPG signing.Consider adding this note before the command:
+Note: The following command uses GPG signing (-s flag). Ensure you have a GPG key configured with Git first. If you haven't set this up, see GitHub's guide on GPG key configuration.
211-211: Format the bare URL as a proper Markdown link.The bare URL should be formatted as a proper Markdown link for better readability and consistency.
-This page contains helpful resources: https://packaging.python.org/en/latest/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows/#configuring-trusted-publishing +This page contains helpful resources: [Trusted Publishing Configuration](https://packaging.python.org/en/latest/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows/#configuring-trusted-publishing)🧰 Tools
🪛 Markdownlint
211-211: null
Bare URL used(MD034, no-bare-urls)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- CONTRIBUTING.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
CONTRIBUTING.md
[style] ~212-~212: Consider using “email”.
Context: ...lows/#configuring-trusted-publishing 4. Send an email to [xrpl-announce](https://groups.google.c...(SEND_AN_EMAIL)
🪛 Markdownlint
CONTRIBUTING.md
211-211: null
Bare URL used(MD034, no-bare-urls)
🔇 Additional comments (1)
CONTRIBUTING.md (1)
216-216: LGTM! Clear distinction between automated and manual processes.The note clearly indicates that the manual release process is optional while preserving the information for maintainers who prefer this approach.
| - name: Load cached .local | ||
| id: cache-poetry | ||
| uses: actions/cache@v3 | ||
| with: | ||
| path: /home/runner/.local | ||
| key: dotlocal-${{ env.POETRY_VERSION }} | ||
| - name: Install poetry |
| if: startsWith(github.ref, 'refs/tags/') # only publish to PyPI on tag pushes | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| id-token: write # IMPORTANT: mandatory for trusted publishing |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
When you say something is mandatory, it’s best to link to why
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- .github/workflows/publish_to_pypi.yml (1 hunks)
- pyproject.toml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
🧰 Additional context used
🪛 actionlint
.github/workflows/publish_to_pypi.yml
25-25: shellcheck reported issue in this script: SC2086:info:2:28: Double quote to prevent globbing and word splitting
(shellcheck)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
For some reason, the CodeRabbit AI reminds me of the bunny in Zootopia :)), but helpful suggestions. Thanks. |
| name: Build distribution 📦 | ||
| runs-on: ubuntu-latest | ||
| env: | ||
| POETRY_VERSION: 1.8.3 |
There was a problem hiding this comment.
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.
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?
| - name: Set up Python | ||
| uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: ">=3.8,<3.13" |
There was a problem hiding this comment.
Don’t we have a file to specify the version of Python?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
.github/workflows/publish_to_pypi.yml (2)
24-25: Document cache key design.
The simple cache key design should be documented to explain why only Poetry version is sufficient.
Add a comment explaining the cache key design:
with:
path: /home/runner/.local
+ # Cache key only includes Poetry version as the cache is Poetry-specific
+ # and independent of Python version or OS
key: dotlocal-${{ env.POETRY_VERSION }}114-116: Improve release upload command formatting.
The command formatting can be improved for better readability and maintainability.
- gh release upload
- '${{ github.ref_name }}' dist/**
- --repo '${{ github.repository }}'
+ gh release upload \
+ '${{ github.ref_name }}' \
+ dist/** \
+ --repo '${{ github.repository }}'📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- .github/workflows/publish_to_pypi.yml (1 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/publish_to_pypi.yml
28-28: shellcheck reported issue in this script: SC2086:info:2:30: Double quote to prevent globbing and word splitting
(shellcheck)
64-64: input "retry-on-error" is not defined in action "pypa/gh-action-pypi-publish@release/v1". available inputs are "attestations", "packages-dir", "packages_dir", "password", "print-hash", "print_hash", "repository-url", "repository_url", "skip-existing", "skip_existing", "user", "verbose", "verify-metadata", "verify_metadata"
(action)
105-105: shellcheck reported issue in this script: SC2215:warning:11:1: This flag is used as a command name. Bad line break or missing [ .. ]?
(shellcheck)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- .github/workflows/publish_to_pypi.yml (1 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/publish_to_pypi.yml
28-28: shellcheck reported issue in this script: SC2086:info:2:30: Double quote to prevent globbing and word splitting
(shellcheck)
66-66: key "verbose" is duplicated in "with" section. previously defined at line:65,col:9. note that this key is case insensitive
(syntax-check)
106-106: shellcheck reported issue in this script: SC2215:warning:11:1: This flag is used as a command name. Bad line break or missing [ .. ]?
(shellcheck)
🪛 yamllint
.github/workflows/publish_to_pypi.yml
[warning] 5-5: wrong indentation: expected 6 but found 8
(indentation)
[warning] 12-12: wrong indentation: expected 6 but found 8
(indentation)
[warning] 15-15: wrong indentation: expected 6 but found 4
(indentation)
[warning] 24-24: wrong indentation: expected 8 but found 10
(indentation)
[warning] 52-52: wrong indentation: expected 6 but found 8
(indentation)
[warning] 54-54: wrong indentation: expected 6 but found 4
(indentation)
[error] 66-66: duplication of key "verbose" in mapping
(key-duplicates)
[warning] 73-73: wrong indentation: expected 6 but found 4
(indentation)
[warning] 78-78: wrong indentation: expected 6 but found 8
(indentation)
[warning] 82-82: wrong indentation: expected 6 but found 4
(indentation)
| with: | ||
| verbose: true | ||
| verbose: true |
There was a problem hiding this comment.
Remove duplicate 'verbose' key.
The 'verbose' key is duplicated in the PyPI publish step configuration.
with:
verbose: true
- verbose: true📝 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.
| with: | |
| verbose: true | |
| verbose: true | |
| with: | |
| verbose: true |
🧰 Tools
🪛 actionlint
66-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)
| gh release upload | ||
| '${{ github.ref_name }}' dist/** | ||
| --repo '${{ github.repository }}' |
There was a problem hiding this comment.
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
‼️ 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.
| gh release upload | |
| '${{ github.ref_name }}' dist/** | |
| --repo '${{ github.repository }}' | |
| gh release upload \ | |
| '${{ github.ref_name }}' \ | |
| dist/** \ | |
| --repo '${{ github.repository }}' |
remove the check-wheel-contents usage; poetry build does not adhere to the required standards of the tool
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
.github/workflows/publish_to_pypi.yml (1)
103-106: Fix release upload command formatting.
The current command formatting could cause issues. Each argument should be on a new line with proper continuation.
- run: >-
- gh release upload
- '${{ github.ref_name }}' dist/**
- --repo '${{ github.repository }}'
+ run: |
+ gh release upload \
+ '${{ github.ref_name }}' \
+ dist/** \
+ --repo '${{ github.repository }}'📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- .github/workflows/publish_to_pypi.yml (1 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/publish_to_pypi.yml
29-29: shellcheck reported issue in this script: SC2086:info:2:30: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (2)
.github/workflows/publish_to_pypi.yml (2)
58-61: 🛠️ Refactor suggestion
Add package metadata verification.
Enable metadata verification to ensure all required PyPI package metadata fields are properly set before publishing.
uses: pypa/gh-action-pypi-publish@release/v1
with:
verbose: true
+ verify-metadata: trueLikely invalid or redundant comment.
25-26: 🛠️ Refactor suggestion
Enhance cache key with poetry.lock hash.
The current cache key only depends on the Poetry version. Including a hash of poetry.lock would ensure cache invalidation when dependencies change.
- key: dotlocal-${{ env.POETRY_VERSION }}
+ key: dotlocal-${{ env.POETRY_VERSION }}-${{ hashFiles('poetry.lock') }}Likely invalid or redundant comment.
| on: | ||
| push: | ||
| tags: | ||
| - '*' |
There was a problem hiding this comment.
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.
on:
push:
tags:
- - '*'
+ - 'v[0-9]+.[0-9]+.[0-9]+*' # Matches semantic version tags like v1.2.3📝 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.
| on: | |
| push: | |
| tags: | |
| - '*' | |
| on: | |
| push: | |
| tags: | |
| - 'v[0-9]+.[0-9]+.[0-9]+*' # Matches semantic version tags like v1.2.3 |
There was a problem hiding this comment.
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.
@ckeshava, understood. I'll note this preference for future reviews.
(_/)
( ._.)
/
✏️ Learnings added
Learnt from: ckeshava
PR: XRPLF/xrpl-py#760
File: .github/workflows/publish_to_pypi.yml:2-5
Timestamp: 2024-10-29T22:43:38.925Z
Learning: In the `.github/workflows/publish_to_pypi.yml` file, when configuring the tag pattern for triggering the release workflow, use the flexible pattern `'*'` instead of restricting to version tags. This allows for releasing specific feature builds (like AMM, MPT, etc.) and provides more flexibility in the tagging process.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@khancode can you please review this PR? |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
.github/workflows/publish_to_pypi.yml (1)
24-26: Enhance cache key specificity.The cache key could be more specific by including the runner OS to prevent potential cache conflicts.
path: /home/runner/.local - key: dotlocal-${{ env.POETRY_VERSION }}-${{ hashFiles('poetry.lock') }} + key: dotlocal-${{ runner.os }}-${{ env.POETRY_VERSION }}-${{ hashFiles('poetry.lock') }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/publish_to_pypi.yml(1 hunks)
🧰 Additional context used
📓 Learnings (1)
.github/workflows/publish_to_pypi.yml (1)
Learnt from: ckeshava
PR: XRPLF/xrpl-py#760
File: .github/workflows/publish_to_pypi.yml:2-5
Timestamp: 2024-10-29T22:43:40.212Z
Learning: In the `.github/workflows/publish_to_pypi.yml` file, when configuring the tag pattern for triggering the release workflow, use the flexible pattern `'*'` instead of restricting to version tags. This allows for releasing specific feature builds (like AMM, MPT, etc.) and provides more flexibility in the tagging process.
🪛 actionlint
.github/workflows/publish_to_pypi.yml
29-29: shellcheck reported issue in this script: SC2086:info:2:30: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (1)
.github/workflows/publish_to_pypi.yml (1)
40-63: LGTM! Well-configured PyPI publishing job.
The job includes all necessary safety measures:
- Explicit dependency on build job
- Timeout configuration
- Proper permissions for trusted publishing
- Artifact verification
| - name: Build a binary wheel and a source tarball | ||
| run: poetry build |
There was a problem hiding this comment.
🛠️ 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.
| - 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 |
| run: >- | ||
| gh release upload | ||
| '${{ github.ref_name }}' dist/** | ||
| --repo '${{ github.repository }}' |
There was a problem hiding this comment.
🛠️ 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
‼️ 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.
| run: >- | |
| gh release upload | |
| '${{ github.ref_name }}' dist/** | |
| --repo '${{ github.repository }}' | |
| # 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 }}' |
High Level Overview of Change
This proposal is a quality-of-life improvement for developers. It seeks to automate the Release process, as detailed in the CONTRIBUTING.md file. It should not affect the experience of the end-users of the library.
If tagged commits are pushed into this repository, it triggers the Github Actions workflow as defined in
publish_to_pypi.ymlfile. If required, it can be modified to run only on pushes to themainbranch.Context of Change
Type of Change
Did you update CHANGELOG.md?
Objectives of this PR:
[x] Github Releases can also be included into the Github Actions script, using the
ghcommand-line utility.Note: The auto-generated notes in the Github Release page are minimalistic. A maintainer needs to manually fill-in the Breaking-Changes, Additions, Fixes sections.
[x] The distribution packages can be signed using
Sigstore, for additional security. This ensures that downstream applications can use trusted deployments ofxrpl-pysoftware distribution.More information can be found here: https://packaging.python.org/en/latest/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows/#signing-the-distribution-packages
The
sigstorehash is published as one of the assets on theReleasepage.[x] Automatically publish the distribution onto PyPI
Test Plan
I have tested the process of publishing the package and upgrading the package at this PyPI test package location: https://pypi.org/project/xrpl-py-release-test.
Future Tasks
Summary by CodeRabbit
New Features
CONTRIBUTING.mdfile, including version incrementing and tagging.Documentation
CONTRIBUTING.mdto clarify the release process and automate steps for maintainers.