Skip to content

Use cibuildwheel and update grpcio #57

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jul 1, 2022
Merged

Use cibuildwheel and update grpcio #57

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6c10d2f
Use cibuildwheel and update grpcio
cretz Jun 28, 2022
0334677
Move compile binary into reusable step
cretz Jun 29, 2022
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
78 changes: 70 additions & 8 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,35 +82,97 @@ jobs:
include:
- os: ubuntu-latest
package-suffix: linux-amd64
ci-arch: auto
- os: macos-latest
package-suffix: macos-amd64
ci-arch: auto
- os: macos-latest
package-suffix: macos-arm64
ci-arch: arm64
rust-add-target: aarch64-apple-darwin
- os: windows-latest
package-suffix: windows-amd64
ci-arch: auto
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v2
with:
submodules: recursive
- uses: actions-rs/toolchain@v1
- uses: actions/setup-python@v1
with:
python-version: "3.10"

# Install Rust locally for non-Linux (Linux uses an internal docker
# command to build with cibuildwheel which uses rustup install defined
# in pyproject.toml)
- if: ${{ runner.os != 'Linux' }}
uses: actions-rs/toolchain@v1
with:
toolchain: stable
- uses: Swatinem/rust-cache@v1
target: ${{ matrix.rust-add-target }}
- if: ${{ runner.os != 'Linux' }}
uses: Swatinem/rust-cache@v1
with:
working-directory: temporalio/bridge

# Prepare
- run: python -m pip install --upgrade wheel poetry poethepoet
- run: poetry install --no-root

# Add the source dist only for Linux x64 for now
- if: ${{ matrix.package-suffix == 'linux-amd64' }}
run: poetry build --format sdist

# Build and fix the wheel
- run: poetry run cibuildwheel --output-dir dist --arch ${{ matrix.ci-arch }}
- run: poe fix-wheel

# Do test only for ci-arch auto (i.e. local machine)
- if: ${{ matrix.ci-arch == 'auto' }}
uses: actions/setup-go@v2
with:
go-version: "1.18"
- if: ${{ matrix.ci-arch == 'auto' }}
run: poe test-dist-single

# Upload dist
- uses: actions/upload-artifact@v2
with:
name: packages-${{ matrix.package-suffix }}
path: dist

# We separate out Linux aarch64 so we can choose not to run it during PR since
# it is so slow in cibuildwheel (uses QEMU emulation). We can put this back in
# the above matrix when Linux ARM runners are available.
compile-binaries-linux-aarch64:
Copy link
Member

Choose a reason for hiding this comment

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

nit: you can merge this with the matrix above and add conditions to skip in PR CI

Copy link
Member Author

@cretz cretz Jun 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

The job if can't include a matrix variable in its expression. It is evaluated before the matrix is loaded. There is also no "exit early" step for a job. Technically I could add an if to each step to skip linux aarch64 during PR, but that is ugly. There are also other ugly options: https://stackoverflow.com/questions/65384420/how-to-make-a-github-action-matrix-element-conditional.

This is the result of several attempts at several options (including workflow reuse which had little value and you can't use a strategy with a job use clause anyways).

(the original version did have these combined before it became clear Linux aarch64 was way too slow, but I still want to keep cibuildwheel's approach instead of custom cross-compile at least if/until we get Linux arm runners)

# Skip compiling Linux aarch64 on PR
if: ${{ github.event_name != 'pull_request' }}
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
with:
submodules: recursive
- uses: actions/setup-python@v1
with:
python-version: "3.10"
# Needed for tests since they use external server
- uses: actions/setup-go@v2

# Need QEMU for ARM build on Linux
- uses: docker/setup-qemu-action@v1
with:
go-version: "1.18"
image: tonistiigi/binfmt:latest
platforms: arm64

# Prepare
- run: python -m pip install --upgrade wheel poetry poethepoet
- run: poetry install --no-root
- run: poetry build

# Build and fix the wheel
- run: poetry run cibuildwheel --output-dir dist --arch aarch64
- run: poe fix-wheel
- run: poe test-dist-single

# Upload dist
- uses: actions/upload-artifact@v2
with:
name: packages-${{ matrix.package-suffix }}
name: packages-linux-aarch64
path: dist

3 changes: 2 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ These steps can be followed to use with a virtual environment and `pip`:

For macOS M1 users on Python 3.10+, due to an
[improperly named wheel in PyPI for gRPC](https://github.com/grpc/grpc/issues/28387), you may have to run
`pip install --no-binary :all: grpcio --ignore-installed` in your environment before use.
`pip install --no-binary :all: grpcio --ignore-installed` in your environment before use. (this is fixed in the next
version of `temporalio` and in the `main` branch)

The SDK is now ready for use. To build from source, see "Building" near the end of this documentation.

Expand Down
Loading