Skip to content
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

Force Poetry setup file #271

Merged
merged 2 commits into from
Feb 13, 2023
Merged

Force Poetry setup file #271

merged 2 commits into from
Feb 13, 2023

Conversation

cretz
Copy link
Member

@cretz cretz commented Feb 9, 2023

What was changed

When originally developing Python SDK, we weren't able to lean on Maturin for our advanced use of PyO3 with Rust (details escape me currently) so we use setuptools-rust to build our wheels. In order to do this, we need to plugin to the traditional setup.py approach that Python builds take which is why we told Poetry to use a build script that let us add setup.py extensions.

Since the latest Poetry core release 3 weeks ago, this approach was turned off by default in python-poetry/poetry-core#318 and started failing our cibuildwheel binary builds because our Rust project wasn't being built anymore.

This PR opts back in to continued setup.py generation. Unfortunately they'll probably remove this option altogether which means we'll either need to: 1) try to use Maturin, 2) find another way to use setuptools-rust, 3) add a build hook in Poetry to manually build extension, or 4) leave Poetry. But that decision is for a later time.

@cretz cretz requested a review from a team February 9, 2023 17:53
@cretz cretz marked this pull request as ready for review February 9, 2023 17:53
# a setup file, but we are using setuptools Rust manually in our build.py which
# needs a setup file.
# TODO(cretz): Find a way to not require a setup file since that is going away
# at some point in Poetry. Revisit Maturin or fine some other approach.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# at some point in Poetry. Revisit Maturin or fine some other approach.
# at some point in Poetry. Revisit Maturin or find some other approach.

@cretz cretz merged commit 1fa36c2 into temporalio:main Feb 13, 2023
@cretz cretz deleted the fix-compile-binaries branch February 13, 2023 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants