Optional msgspec support#214
Merged
Merged
Conversation
Owner
Author
|
So turns out those last CI runs never had This demonstrates 2 interesting outcomes
Lots more work to do here but quick summary is I'm going to defer this support and pull out the transport stuff that's needed for the bi-dir streaming changes and put it as a separate smaller PR onto mainline. |
goodboy
force-pushed
the
optional_msgspec_support
branch
from
July 2, 2021 15:25
a25dfc7 to
94784d2
Compare
goodboy
force-pushed
the
optional_msgspec_support
branch
2 times, most recently
from
September 5, 2021 22:28
7f89f9a to
c27b006
Compare
Owner
Author
|
Welp, now all the non- Will have to dig into it. |
Merged
goodboy
force-pushed
the
optional_msgspec_support
branch
from
September 8, 2021 13:53
e8b2828 to
4e61fd1
Compare
goodboy
force-pushed
the
optional_msgspec_support
branch
2 times, most recently
from
October 4, 2021 16:40
75eb4dd to
fbc6905
Compare
Can only really use an encoder currently since there is no streaming api in `msgspec` as of currently. See msgspec/msgspec#27. Not sure if any encoding speedups are currently noticeable especially without any validation going on yet XD. First experiments toward #196
Add a `tractor._ipc.MsgspecStream` type which can be swapped in for `msgspec` serialization transparently. A small msg-length-prefix framing is implemented as part of the type and we use `tricycle.BufferedReceieveStream` to handle buffering logic for the underlying transport. Notes: - had to force cast a few more list -> tuple spots due to no native `tuple`decode-by-default in `msgspec`: msgspec/msgspec#30 - the framing can be understood by this protobuf walkthrough: https://eli.thegreenplace.net/2011/08/02/length-prefix-framing-for-protocol-buffers - `tricycle` becomes a new dependency
This change some super old (and bad) code from the project's very early days. For some redic reason i must have thought masking `trio`'s internal stream / transport errors and a TCP EOF as `StopAsyncIteration` somehow a good idea. The reality is you probably want to know the difference between an unexpected transport error and a simple EOF lol. This begins to resolve that by adding our own special `TransportClosed` error to signal the "graceful" termination of a channel's underlying transport. Oh, and this builds on the `msgspec` integration which helped shed light on the core issues here B)
`msgspec` sends python lists over the wire (msgspec/msgspec#30) which is fine and dandy but we use them as lookup keys so we need to be sure we tuple-cast first.
goodboy
force-pushed
the
optional_msgspec_support
branch
from
October 5, 2021 17:37
fbc6905 to
135459c
Compare
In an effort to have some kind of more formal interface around the transport layer, add a `MsgTransport` protocol type and use with the channel composition of message streams. Start a little "key map" of `(<codec>, <protocol>)` to `MsgTransport` types which can be dynamically loaded. Add a `Channel.from_stream()` constructor thus cleaning up the mangled logic that was in the constructor based on inputs. Drop all the "auto reconnect" channel logic for now since nothing is using it (internally) and it's likely it will need rework once we bring in a protocol besides TCP.
Merged
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Supersedes #212 (mostly for git history reasons) but adding
msgspecas an optional dependency which is loaded as the transport backendTCPMsgspecStreamif importable.This also reworks TCP transport EOF handling to raise a new internal error type
TransportClosedwhich now handled around the core message loop and inter-actor handshake.This also add
tricycleas a dep since it has a lot useful stuff anyway that we'll probably use and once we go fullmsgspecafter dropping<= 3.8python, it'll be necessary.TODO list from #212:
defaultdicts for some reason as well as tuple assertion issues for channel uuidsadd nested structs for our ipc protocol for Spec out the async-ipc protocol #36deferring for now..start_methodkwarg toopen_root_actor()), maybeserialization_method?mypystuffpotentially offerwas already dropped.msgpack-numpyas the optional dep if we end up liking the performance here?msgspecrequires it) and we might as well sine we're onto a new alpha release anywaymsgspecuse in the readme