Skip to content

Check that intended python version is being used #808

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

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/build-binaries.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ jobs:
with:
workspaces: temporalio/bridge -> target
- uses: astral-sh/setup-uv@v5
- run: uv sync --all-extras
- run: uv sync --all-extras --python 3.13

# Add the source dist only for Linux x64 for now
- if: ${{ matrix.package-suffix == 'linux-amd64' }}
Expand Down
17 changes: 15 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,20 @@ jobs:
repo-token: ${{ secrets.GITHUB_TOKEN }}
- uses: astral-sh/setup-uv@v5
- run: uv tool install poethepoet
- run: uv sync --all-extras
- run: uv sync --all-extras --python ${{ matrix.python }}
Copy link
Member

Choose a reason for hiding this comment

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

What is the use case for --python instead of letting it use the existing installed one? (even if we change the install method)

- name: Check python version
shell: bash
run: |
actual=$(uv run python -c "import sys; print('.'.join(map(str, sys.version_info[:2])))")
if [[ "$actual" != "${{ matrix.python }}" ]]; then
echo "Python version in use by uv ($actual) does not match intended version (${{ matrix.python }})"
exit 1
fi
actual=$(python -c "import sys; print('.'.join(map(str, sys.version_info[:2])))")
if [[ "$actual" != "${{ matrix.python }}" ]]; then
echo "Python version on PATH ($actual) does not match intended version (${{ matrix.python }})"
exit 1
fi
Comment on lines +52 to +64
Copy link
Member

@cretz cretz Apr 1, 2025

Choose a reason for hiding this comment

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

Is this embedded GH workflow logic/check necessary? Can we not trust setup-python? Do we need to check Rust version, protoc version, CPU arch, etc too if we're concerned with the actions? I can't tell from the PR description whether something happened that made this not work and made us concerned.

- run: poe bridge-lint
if: ${{ matrix.clippyLinter }}
- run: poe lint
Expand Down Expand Up @@ -84,7 +97,7 @@ jobs:
TEMPORAL_TEST_PROTO3: 1
run: |
uv add --python 3.9 "protobuf<4"
uv sync --all-extras
uv sync --all-extras --python 3.9
poe build-develop
poe gen-protos
poe format
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/run-bench.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ jobs:
- uses: astral-sh/setup-uv@v5
# Build
- run: uv tool install poethepoet
- run: uv sync --all-extras
- run: uv sync --all-extras --python 3.13
- run: poe build-develop-with-release

# Run a bunch of bench tests. We run multiple times since results vary.
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1544,7 +1544,7 @@ poe test -s --log-cli-level=DEBUG -k test_sync_activity_thread_cancel_caught
#### Proto Generation and Testing

To allow for backwards compatibility, protobuf code is generated on the 3.x series of the protobuf library. To generate
protobuf code, you must be on Python <= 3.10, and then run `uv add "protobuf<4"` + `uv sync --all-extras`. Then the
protobuf code, run `uv add "protobuf<4" --python 3.10` + `uv sync --all-extras --python 3.10`. Then the
protobuf files can be generated via `poe gen-protos`. Tests can be run for protobuf version 3 by setting the
`TEMPORAL_TEST_PROTO3` env var to `1` prior to running tests.

Expand Down
Loading