Copilot commented on code in PR #3598:
URL: https://github.com/apache/thrift/pull/3598#discussion_r3413021353


##########
doc/ReleaseManagement.md:
##########
@@ -432,8 +432,23 @@ See https://thrift.apache.org/lib/ for the current status 
of each external packa
     increase the suffix. (_1, _2, ...) and upload another.  You cannot replace 
a release on CPAN.
 * [php] @jfarrell, @bufferoverflow, @jeking3 are the only ones who can do this 
right now.
   * Once the release is tagged, one just has to hit the "Update" button to 
pick it up.
-* [pypi] @jfarrell is the only one who can do this right now.
-    https://issues.apache.org/jira/browse/THRIFT-4687
+* [pypi] The `PyPI publishing` GitHub Actions workflow publishes the Python
+  package when the GitHub release is published. It builds the source

Review Comment:
   This states the workflow publishes when a GitHub release is published, but 
the workflow currently only publishes for non-prerelease releases (it skips 
when `github.event.release.prerelease` is true). Please clarify in the doc that 
prereleases are excluded (and that publishing is gated accordingly), so the 
release procedure matches the actual workflow behavior.



##########
.github/workflows/pypi.yml:
##########
@@ -22,16 +22,26 @@ name: "PyPI publishing"
 on:
   release:
     types: [published]
+  pull_request:
+    branches:
+      - master

Review Comment:
   The PR trigger is configured for the `master` branch, but this PR appears to 
target `main` (per diff base). As written, the workflow won’t run for PRs 
opened against `main`. Update `pull_request.branches` to include `main` (or 
match the repository’s default branch) so CI validation runs as intended.



##########
.github/workflows/pypi.yml:
##########
@@ -40,14 +50,126 @@ jobs:
       - name: Set up Python
         uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # 
v6.2.0
         with:
-          python-version: "3.8"
+          python-version: "3.12"
+
+      - name: Install build tools
+        run: python -m pip install --upgrade build twine
+
+      - name: Build source distribution
+        working-directory: lib/py
+        run: python -m build --sdist --outdir dist
+
+      - name: Check source distribution
+        working-directory: lib/py
+        run: python -m twine check dist/*
+
+      - name: Upload source distribution
+        uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a 
# v7.0.1
+        with:
+          name: python-sdist
+          if-no-files-found: error
+          path: lib/py/dist/*.tar.gz
+          retention-days: 3
+
+  build-wheels:
+    name: Build Python wheels (${{ matrix.platform }})
+    runs-on: ${{ matrix.runner }}
+    timeout-minutes: 90
+    permissions:
+      contents: read
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+          - platform: manylinux-x86_64
+            runner: ubuntu-latest
+            archs: x86_64
+            build: "cp310-manylinux_* cp311-manylinux_* cp312-manylinux_* 
cp313-manylinux_* cp314-manylinux_*"
+          - platform: manylinux-aarch64
+            runner: ubuntu-24.04-arm
+            archs: aarch64
+            build: "cp310-manylinux_* cp311-manylinux_* cp312-manylinux_* 
cp313-manylinux_* cp314-manylinux_*"
+          - platform: musllinux-x86_64
+            runner: ubuntu-latest
+            archs: x86_64
+            build: "cp310-musllinux_* cp311-musllinux_* cp312-musllinux_* 
cp313-musllinux_* cp314-musllinux_*"
+          - platform: musllinux-aarch64
+            runner: ubuntu-24.04-arm
+            archs: aarch64
+            build: "cp310-musllinux_* cp311-musllinux_* cp312-musllinux_* 
cp313-musllinux_* cp314-musllinux_*"
+          - platform: macos-x86_64
+            runner: macos-15-intel
+            archs: x86_64
+            build: "cp310-macosx_* cp311-macosx_* cp312-macosx_* 
cp313-macosx_* cp314-macosx_*"
+          - platform: macos-arm64
+            runner: macos-15
+            archs: arm64
+            build: "cp310-macosx_* cp311-macosx_* cp312-macosx_* 
cp313-macosx_* cp314-macosx_*"
+          - platform: windows-amd64
+            runner: windows-latest
+            archs: AMD64
+            build: "cp310-win_* cp311-win_* cp312-win_* cp313-win_* 
cp314-win_*"
+    steps:
+      - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # 
v6.0.2
+        with:
+          persist-credentials: false
+
+      - name: Set up Python
+        uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # 
v6.2.0
+        with:
+          python-version: "3.12"
+
+      - name: Install build tools
+        run: python -m pip install --upgrade cibuildwheel==4.1.0 twine
+
+      - name: Build wheels
+        run: python -m cibuildwheel lib/py --output-dir wheelhouse
+        env:
+          CIBW_ARCHS: ${{ matrix.archs }}
+          CIBW_BUILD: ${{ matrix.build }}
+          CIBW_MANYLINUX_X86_64_IMAGE: manylinux2014
+          CIBW_MANYLINUX_AARCH64_IMAGE: manylinux2014
+          CIBW_TEST_COMMAND: python -c "import thrift; import 
thrift.protocol.fastbinary"
+
+      - name: Check wheels
+        run: >
+          python -c "import glob, subprocess, sys;
+          sys.exit(subprocess.call([sys.executable, '-m', 'twine', 'check',
+          *glob.glob('wheelhouse/*.whl')]))"
+
+      - name: Upload wheels
+        uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a 
# v7.0.1
+        with:
+          name: python-wheels-${{ matrix.platform }}
+          if-no-files-found: error
+          path: wheelhouse/*.whl
+          retention-days: 3
+
+  pypi-publish:
+    name: Publish release to PyPI
+    needs:
+      - build-sdist
+      - build-wheels
+    if: ${{ github.event_name == 'release' && !github.event.release.prerelease 
}}

Review Comment:
   The publish gate relies solely on the GitHub Release `prerelease` flag. This 
doesn’t enforce the documented policy of not uploading release 
candidates/prereleases if someone forgets to mark the release as a prerelease. 
Consider additionally gating on the tag name (e.g., reject PEP 440 pre-release 
segments like `a`, `b`, `rc`, `.dev`) or requiring a manual approval via the 
`release` environment for extra safety.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to