-
Notifications
You must be signed in to change notification settings - Fork 83
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
uv + maturin migration #768
Merged
Merged
Changes from all commits
Commits
Show all changes
38 commits
Select commit
Hold shift + click to select a range
829845e
uvx migrate-to-uv
dandavison 4b14fb3
Reinstate comments
dandavison 34ef739
Fix license: `poe build-develop` works
dandavison 0bd94a9
Pin mypy
dandavison 0dd9410
Migrate CI and README
dandavison c8c08bc
Revert build system changes
dandavison 8a07c88
Fix type errors
dandavison dbb54e0
Pin pyright
dandavison ca4a8c5
Partial revert
dandavison e76eb5c
tool.uv package = false
dandavison 58dd5a3
Use uv for poe tasks
dandavison 00534fa
Use features branch
dandavison 1a21c69
Get rid of single-test poe task
dandavison 38f0064
Do not use `--no-install-project`.
dandavison c90f0a3
Migrate CI and README
dandavison 1eeb976
Use maturin
dandavison 6da07fe
s/temporalio.bridge.temporal_sdk_bridge/rg-replace temporal_sdk_bridge/
dandavison 3c7ec74
maturin versions of commands
dandavison 43d2a2f
Include temporalio
dandavison 12ef4d4
Install maturin as a dev dependency
dandavison 6935476
Need to explicitly install the rust extension now
dandavison 222817f
Revert "s/temporalio.bridge.temporal_sdk_bridge/rg-replace temporal_s…
dandavison 0d98712
Set module-name
dandavison a7f3b50
Get rid of fix-wheel hacks
dandavison a36040f
Use `uv tool` to install poe
dandavison de3b55e
Add Cargo.toml metadata
dandavison e7ea8f1
fixup uv.lock after rebase
dandavison 670e0d2
TEMP: Remove warnings
dandavison 6246db0
Get rid of redundant `uv build`
dandavison b114ccd
Smoke test
dandavison 83a82ef
DEV: run build-binaries workflow
dandavison ba38732
Windows
dandavison bc54894
Revert "TEMP: Remove warnings"
dandavison 962a055
MANIFEST.in is having no effect
dandavison 805e674
Tweak wheel contents
dandavison 40e21f2
Run entire test suite
dandavison ec4557f
Revert "Run entire test suite"
dandavison f730e85
Revert "DEV: run build-binaries workflow"
dandavison File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1390,28 +1390,27 @@ The Python SDK is built to work with Python 3.9 and newer. It is built using | |
|
||
To build the SDK from source for use as a dependency, the following prerequisites are required: | ||
|
||
* [Python](https://www.python.org/) >= 3.9 | ||
* Make sure the latest version of `pip` is in use | ||
* [uv](https://docs.astral.sh/uv/) | ||
* [Rust](https://www.rust-lang.org/) | ||
* [Protobuf Compiler](https://protobuf.dev/) | ||
* [poetry](https://github.com/python-poetry/poetry) (e.g. `python -m pip install poetry`) | ||
* [poe](https://github.com/nat-n/poethepoet) (e.g. `python -m pip install poethepoet`) | ||
|
||
macOS note: If errors are encountered, it may be better to install Python and Rust as recommended from their websites | ||
instead of via `brew`. | ||
Use `uv` to install `poe`: | ||
|
||
With the prerequisites installed, first clone the SDK repository recursively: | ||
```bash | ||
uv tool install poethepoet | ||
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. We'll probably switch away from |
||
``` | ||
|
||
Now clone the SDK repository recursively: | ||
|
||
```bash | ||
git clone --recursive https://github.com/temporalio/sdk-python.git | ||
cd sdk-python | ||
``` | ||
|
||
Use `poetry` to install the dependencies with `--no-root` to not install this package (because we still need to build | ||
it): | ||
Install the dependencies: | ||
|
||
```bash | ||
poetry install --no-root --all-extras | ||
uv sync --all-extras | ||
``` | ||
|
||
#### Build | ||
|
@@ -1422,16 +1421,11 @@ Now perform the release build: | |
environment](#local-sdk-development-environment) for the quicker approach to local development). | ||
|
||
```bash | ||
poetry build | ||
uv build | ||
``` | ||
|
||
The compiled wheel doesn't have the exact right tags yet for use, so run this script to fix it: | ||
|
||
```bash | ||
poe fix-wheel | ||
``` | ||
|
||
The `whl` wheel file in `dist/` is now ready to use. | ||
The `.whl` wheel file in `dist/` is now ready to use. | ||
|
||
#### Use | ||
|
||
|
@@ -1487,22 +1481,15 @@ It should output: | |
|
||
### Local SDK development environment | ||
|
||
For local development, it is often quicker to use debug builds and a local virtual environment. | ||
|
||
While not required, it often helps IDEs if we put the virtual environment `.venv` directory in the project itself. This | ||
can be configured system-wide via: | ||
|
||
```bash | ||
poetry config virtualenvs.in-project true | ||
``` | ||
For local development, it is quicker to use a debug build. | ||
|
||
Now perform the same steps as the "Prepare" section above by installing the prerequisites, cloning the project, | ||
installing dependencies, and generating the protobuf code: | ||
Perform the same steps as the "Prepare" section above by installing the prerequisites, cloning the project, and | ||
installing dependencies: | ||
|
||
```bash | ||
git clone --recursive https://github.com/temporalio/sdk-python.git | ||
cd sdk-python | ||
poetry install --no-root --all-extras | ||
uv sync --all-extras | ||
``` | ||
|
||
Now compile the Rust extension in develop mode which is quicker than release mode: | ||
|
@@ -1535,14 +1522,14 @@ 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 `poetry add "protobuf<4"` + | ||
`poetry install --no-root --all-extras`. 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. | ||
protobuf code, you must be on Python <= 3.10, and then run `uv add "protobuf<4"` + `uv sync --all-extras`. 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. | ||
|
||
Do not commit `poetry.lock` or `pyproject.toml` changes. To go back from this downgrade, restore both of those files | ||
and run `poetry install --no-root --all-extras`. Make sure you `poe format` the results. | ||
Do not commit `uv.lock` or `pyproject.toml` changes. To go back from this downgrade, restore both of those files and run | ||
`uv sync --all-extras`. Make sure you `poe format` the results. | ||
|
||
For a less system-intrusive approach, you can (note this approach [may have a bug](https://github.com/temporalio/sdk-python/issues/543)): | ||
For a less system-intrusive approach, you can: | ||
```shell | ||
docker build -f scripts/_proto/Dockerfile . | ||
docker run --rm -v "${PWD}/temporalio/api:/api_new" -v "${PWD}/temporalio/bridge/proto:/bridge_new" <just built image sha> | ||
|
@@ -1552,7 +1539,7 @@ poe format | |
### Style | ||
|
||
* Mostly [Google Style Guide](https://google.github.io/styleguide/pyguide.html). Notable exceptions: | ||
* We use [Black](https://github.com/psf/black) for formatting, so that takes precedence | ||
* We use [ruff](https://docs.astral.sh/ruff/) for formatting, so that takes precedence | ||
* In tests and example code, can import individual classes/functions to make it more readable. Can also do this for | ||
rarely in library code for some Python common items (e.g. `dataclass` or `partial`), but not allowed to do this for | ||
any `temporalio` packages (except `temporalio.types`) or any classes/functions that aren't clear when unqualified. | ||
|
This file was deleted.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Can we get rid of poe? Does uv offer simple ways to define tasks?
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.
Was just commenting on the same thing -- we'll be able to switch away from
poe
whenuv
includes a task runner, which it sounds to me like will happen in next few months.