Packaging, dependency updates, and other errata #8

cretz · 2022-03-14T16:04:46Z

What was changed

Remove Maturin and use setuptools-rust instead
Several hacks around Poetry's limited support for a lot of what we want to do
Update some dependencies which affected the typing checks
Setup CI to build artifacts
Add basic documentation to README
Make sure multiprocess and threaded heartbeating confirms heartbeat receipt

Sushisource · 2022-03-14T17:02:59Z

scripts/fix_wheel.py

+if __name__ == "__main__":
+    # Due to https://github.com/python-poetry/poetry/issues/3509 and Poetry
+    # assuming the tags, we have to change the wheel ourselves after build


Well this is too bad.

Can we just build the wheels directly rather than via poetry? Will that still have this issue, or is it too inconvenient to do in the first place?

I kinda am, but you need Poetry to get all the files that you want in the wheel setup properly and to have the dependency versions set in the dist info properly. (otherwise it gets real hacky)

scripts/setup_bridge.py

README.md

scripts/fix_wheel.py

bergundy · 2022-03-15T20:21:09Z

temporalio/bridge/worker.py

+
+
+def from_bridge_payloads(
+    payloads: Iterable[temporalio.bridge.proto.common.Payload],


@Sushisource we should have just used the API definition of Payload, lang shouldn't need to bother with these conversions.

Yeah, probably, at this point. I am not a fan of the way it's defined but we're stuck with it so may as well get rid of the extra indirection.

cretz · 2022-03-15T20:07:27Z

.github/workflows/ci.yml

@@ -13,7 +13,7 @@ jobs:
      fail-fast: true
      matrix:
        python: ["3.7", "3.10"]
-        os: [ubuntu-latest] # TODO: macos-latest, windows-latest
+        os: [ubuntu-latest, macos-latest, windows-latest]


Opened issue #9 for macOS arm support and #10 for more generic manylinux support

cretz · 2022-03-15T20:10:39Z

pyproject.toml

 description = "Temporal.io Python SDK"
 authors = ["Temporal Technologies Inc <sdk@temporal.io>"]
 license = "MIT"
+# We need to include proto source that is otherwise excluded via .gitignore.


I still wish we didn't exclude proto files from being in-repo. It it so annoying for a user, if they want to see the proto types/files, to have to download a prebuilt artifact and extract it because we won't commit the source.

They're in-repo via the core submodule though, no?

cretz · 2022-03-15T20:12:10Z

README.md

+Install the `temporalio` package from [PyPI](https://pypi.org/project/temporalio). If using `pip` directly, this might
+look like:
+
+    python -m pip install temporalio


I plan on publishing this once we merge

cretz · 2022-03-15T20:13:38Z

temporalio/worker.py

+        # If there is a manager here, we make it flush all pending heartbeats in
+        # the queue
+        if manager:
+            manager.flush_pending_heartbeats(task_token)


This is required for multiprocess activities because it is technically possible some heartbeats remain in queue unprocessed

cretz · 2022-03-15T20:16:19Z

temporalio/worker.py

                logger.exception("Failed during multiprocess queue poll for heartbeat")
                return

+    def flush_pending_heartbeats(self, task_token: bytes) -> None:
+        # We're gonna go ahead and just flush them all


This is admittedly lazy and can result in issues if the queue fills up faster than it can be processed. The processing should be quick since it's just scheduling heartbeats with asyncio, not even converting them or anything.

For the next phase when workflow workers come about, I will probably move all the activity stuff to its own private file/package. I may also provide an alternative SharedStateManager that is a local loopback gRPC server instead of using multiprocessing.Manager() to see if that works better.

cretz · 2022-03-15T20:29:04Z

tests/test_worker.py

+    temporalio.worker.SharedStateManager.create_from_multiprocessing(
+        multiprocessing.Manager()
+    )
+)


We made this eager at the top of tests because where we originally had it, as a local inside a function, was causing all local vars to be copied during fork (called implicitly by multiprocessing.Manager) which included the Python client which has a reference to the Rust client and when that Python client was GC'd from the child process, the Rust client was dropped, causing a file descriptor mismatch.

I have opened #11 to be more explicit about how fork works with this SDK.

cretz · 2022-03-16T19:20:25Z

Closing and reopening to attempt to trigger the actions to get going again (they are stuck for whatever reason)

cretz · 2022-03-17T21:19:02Z

Merging

cretz force-pushed the package-updates branch 3 times, most recently from b6eec40 to 1734194 Compare March 14, 2022 16:38

Sushisource reviewed Mar 14, 2022

View reviewed changes

cretz force-pushed the package-updates branch 13 times, most recently from 8e0cb94 to 23dd21f Compare March 15, 2022 14:26

Work on heartbeats, docs, packaging, and upgrades w/ type fixes

aadb756

cretz force-pushed the package-updates branch from 23dd21f to aadb756 Compare March 15, 2022 18:11

cretz requested a review from a team March 15, 2022 19:54

cretz marked this pull request as ready for review March 15, 2022 19:54

bergundy approved these changes Mar 15, 2022

View reviewed changes

cretz commented Mar 15, 2022

View reviewed changes

cretz force-pushed the package-updates branch 3 times, most recently from e25f0a2 to 9fc91ec Compare March 16, 2022 14:29

Core update and minor improvements

b497c9f

cretz force-pushed the package-updates branch 3 times, most recently from fcbac18 to 8656cc1 Compare March 16, 2022 19:17

cretz closed this Mar 16, 2022

cretz reopened this Mar 16, 2022

cretz force-pushed the package-updates branch 6 times, most recently from 58c1bf7 to 16d6299 Compare March 17, 2022 17:18

Updates for multiprocess heartbeat, packaging, and CI fixes

1008b8c

cretz force-pushed the package-updates branch from 16d6299 to 1008b8c Compare March 17, 2022 19:27

Set alpha version number

55db138

cretz merged commit 641efe8 into temporalio:main Mar 17, 2022

cretz deleted the package-updates branch March 17, 2022 21:19



		def from_bridge_payloads(
		payloads: Iterable[temporalio.bridge.proto.common.Payload],

Packaging, dependency updates, and other errata #8

Packaging, dependency updates, and other errata #8

Uh oh!

Conversation

cretz commented Mar 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What was changed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cretz Mar 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cretz commented Mar 16, 2022

Uh oh!

cretz commented Mar 17, 2022

Uh oh!

Uh oh!

cretz commented Mar 14, 2022 •

edited

Loading

cretz Mar 14, 2022 •

edited

Loading