-
Notifications
You must be signed in to change notification settings - Fork 79
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
Drop Python 3.8, add 3.13 #694
Changes from all commits
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 |
---|---|---|
|
@@ -12,30 +12,23 @@ jobs: | |
strategy: | ||
fail-fast: false | ||
matrix: | ||
python: ["3.8", "3.12"] | ||
python: ["3.9", "3.13"] | ||
os: [ubuntu-latest, ubuntu-arm, macos-intel, macos-arm, windows-latest] | ||
include: | ||
- os: ubuntu-latest | ||
python: "3.12" | ||
python: "3.13" | ||
docsTarget: true | ||
cloudTestTarget: true | ||
clippyLinter: true | ||
- os: ubuntu-latest | ||
python: "3.8" | ||
python: "3.9" | ||
protoCheckTarget: true | ||
- os: ubuntu-arm | ||
runsOn: ubuntu-24.04-arm64-2-core | ||
- os: macos-intel | ||
runsOn: macos-13 | ||
- os: macos-arm | ||
runsOn: macos-14 | ||
# macOS ARM 3.8 does not have an available Python build at | ||
# https://raw.githubusercontent.com/actions/python-versions/main/versions-manifest.json. | ||
# See https://github.com/actions/setup-python/issues/808 and | ||
# https://github.com/actions/python-versions/pull/259. | ||
exclude: | ||
- os: macos-arm | ||
python: "3.8" | ||
runsOn: macos-latest | ||
runs-on: ${{ matrix.runsOn || matrix.os }} | ||
steps: | ||
- uses: actions/checkout@v4 | ||
|
@@ -47,17 +40,13 @@ jobs: | |
workspaces: temporalio/bridge -> target | ||
- uses: actions/setup-python@v5 | ||
with: | ||
# Pinning due to failed Windows builds on 3.12.5 https://github.com/temporalio/sdk-python/issues/637 | ||
python-version: ${{ matrix.python == '3.12' && '3.12.4' || matrix.python }} | ||
python-version: ${{ matrix.python }} | ||
- uses: arduino/setup-protoc@v3 | ||
with: | ||
# TODO(cretz): Can upgrade proto when https://github.com/arduino/setup-protoc/issues/99 fixed | ||
version: "23.x" | ||
repo-token: ${{ secrets.GITHUB_TOKEN }} | ||
# Using fixed Poetry version until | ||
# https://github.com/python-poetry/poetry/issues/7611 and | ||
# https://github.com/python-poetry/poetry/pull/7694 are fixed | ||
- run: python -m pip install --upgrade wheel "poetry==1.3.2" poethepoet | ||
- run: python -m pip install --upgrade wheel poetry poethepoet | ||
- run: poetry install --no-root --all-extras | ||
- run: poe bridge-lint | ||
if: ${{ matrix.clippyLinter }} | ||
|
@@ -93,7 +82,8 @@ jobs: | |
env: | ||
TEMPORAL_TEST_PROTO3: 1 | ||
run: | | ||
poetry add "protobuf<4" | ||
poetry add --python 3.9 "protobuf<4" | ||
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. Worth a comment explaining the 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 admit I do not remember exactly, but I think it was saying there wasn't compatibility with some of the other dependencies |
||
poetry install --no-root --all-extras | ||
poe gen-protos | ||
poe format | ||
[[ -z $(git status --porcelain temporalio) ]] || (git diff temporalio; echo "Protos changed"; exit 1) | ||
|
@@ -122,3 +112,4 @@ jobs: | |
python-repo-path: ${{github.event.pull_request.head.repo.full_name}} | ||
version: ${{github.event.pull_request.head.ref}} | ||
version-is-repo-ref: true | ||
features-repo-ref: python-version-upgrade |
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.
So just to confirm, the
-cp38-
part of the wheel name in PyPi means it is compatible with 3.8+, but in fact the wheel was built using 3.12 (and will now be built with 3.13 and will contain a-cp39-
name fragment).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.
We tell cibuildwheel to build for these minimums:
cp39-win_amd64
,cp39-manylinux_x86_64
,cp39-manylinux_aarch64
,cp39-macosx_x86_64
, andcp39-macosx_arm64
. And we tell PyO3 to compile with ABI compat toabi3-py39
. It is those two tools that make sure during the build-binary step to compile for 3.9 minimum. cibuildwheel sometimes runs inside a container, and it's a certain container they build within to get older libc compat and such (ref https://github.com/pypa/manylinux for Linux for instance). May have to reference those projects for more details or look at the logs of the builds at https://github.com/temporalio/sdk-python/actions/workflows/build-binaries.yml.