Add Python SDK publish workflow#247
Conversation
volcano-sh-bot
added
do-not-merge/work-in-progress
kind/enhancement
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the Python SDK build process in the Makefile to ensure the temporary LICENSE file is removed even if the build fails, excludes Python cache files from the package manifest, and simplifies the license specification in pyproject.toml. Feedback suggests using a shell trap in the Makefile for a more idiomatic and robust cleanup of the temporary license file.
Signed-off-by: Zhou Zihang <z@mcac.cc>
acsoto
force-pushed
the
issue-246-python-sdk-publish
branch
from
7eb0d75 to
6a2be5b
Compare
|
Pls take a look thanks |
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds a GitHub Actions workflow to automatically publish the sdk-python Python SDK to PyPI using trusted publishing (OIDC), alongside small packaging/build tweaks to ensure correct artifacts are produced.
Changes:
- Added
python-sdk-publish.ymlworkflow to build, check PyPI for an existing version, and publish only when needed. - Tightened Python sdist contents by excluding Python cache artifacts.
- Updated the
build-python-sdkMakefile target to use a temporary copiedLICENSEfile with reliable cleanup.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
sdk-python/pyproject.toml |
Updates license metadata format for the SDK package. |
sdk-python/MANIFEST.in |
Excludes __pycache__ and *.py[cod] from the source distribution. |
Makefile |
Makes build-python-sdk copy/remove LICENSE via an EXIT trap for safer cleanup. |
.github/workflows/python-sdk-publish.yml |
New workflow to build the SDK and publish to PyPI via trusted publishing when the version is not already present. |
| id: package | ||
| working-directory: sdk-python | ||
| run: | | ||
| version="$(python -c 'import tomllib; print(tomllib.load(open("pyproject.toml", "rb"))["project"]["version"])')" |
There was a problem hiding this comment.
IIRC, this is using same version for all the changes on main branch right?
There was a problem hiding this comment.
Yes. We use the version declared in sdk-python/pyproject.toml, so if that version does not change, later runs on main will skip
There was a problem hiding this comment.
The question is whether we need automatic version updates or manual version control in sdk-python/pyproject.toml. I think the latter is more under control
| status="$(curl -s -o /dev/null -w "%{http_code}" "https://pypi.org/pypi/${PACKAGE_NAME}/${PACKAGE_VERSION}/json")" | ||
| if [ "$status" = "200" ]; then | ||
| echo "exists=true" >> "$GITHUB_OUTPUT" | ||
| echo "Version ${PACKAGE_VERSION} already exists on PyPI, skipping publish." |
There was a problem hiding this comment.
By default, use ‘latest’ as the release version. Since running this GitHub Action indicates that changes have been made to the python-sdk package in agentCube, it should be updated to the ‘latest’ version number even if the version remains the same.
There was a problem hiding this comment.
Therefore, the ‘version=latest’ should be push unconditionally, followed by the ‘version=x.y.z’ where applicable; to ensure that each agentCube version has the corresponding Python SDK version.
There was a problem hiding this comment.
PyPI package versions are immutable, so unlike container images we cannot keep updating a mutable latest tag for the same package version. Re-publishing the same version is not supported. So if we want to do that we need to do a version auto updater
There was a problem hiding this comment.
However, I think that for the release security, we should explicitly control the version in sdk-python/pyproject.toml
There was a problem hiding this comment.
So, is the best practice for PyPI to push a Python SDK version each time an agentCube release is published?
There was a problem hiding this comment.
I think so. When releasing the agentcube version, manually update the SDK version number
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hzxuzhonghu The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind enhancement
What this PR does / why we need it:
This PR adds an automated publishing workflow for the Python SDK.
The new workflow builds the package from
sdk-python, reads the package version fromsdk-python/pyproject.toml, checks whether that version already exists on PyPI, and only publishes when the version is not already present.It also tightens the SDK packaging flow by:
make build-python-sdkpath so the rootLICENSEremains the single source of truth while still being included in built artifactsTrusted publishing for PyPI has already been configured for this repository, so this workflow does not require a stored PyPI API token. It uses PyPI's trusted publisher flow via GitHub Actions OIDC and
pypa/gh-action-pypi-publish.References:
Which issue(s) this PR fixes:
Fixes #246
Special notes for your reviewer:
The publish job is intentionally limited to
pushonmainand manual dispatch. Existing Python test workflows remain responsible for test coverage; this workflow focuses on build-and-publish behavior.Does this PR introduce a user-facing change?: