- Notifications
You must be signed in to change notification settings - Fork12
Static msg types usingmsgspec
#311
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Draft
goodboy wants to merge2 commits intomainChoose a base branch frommsgtypes
base:main
Could not load branches
Branch not found:{{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline, and old review comments may become outdated.
Draft
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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
goodboy added a commit to pikers/piker that referenced this pull requestJul 7, 2022
Syncs withgoodboy/tractor#311which is nowhere near ready and this approach didn't end up beingas straight forward as hoped. We're going to need a top level`Msg`-boxing type/protocol in `tractor` first...
goodboy added a commit to pikers/piker that referenced this pull requestJul 8, 2022
Syncs withgoodboy/tractor#311which is nowhere near ready and this approach didn't end up beingas straight forward as hoped. We're going to need a top level`Msg`-boxing type/protocol in `tractor` first...
This was referencedJul 8, 2022
8cd8eb5
to54af0d4
Compared1ddfa4
to2d387f2
CompareNeeds a rebase onto master. |
The greasy details are strewn throughout a `msgspec` issue:jcrist/msgspec#140and specifically this code was mostly written as part of POC example inthis comment:jcrist/msgspec#140 (comment)This work obviously pertains to our desire and prep for typed messagingand capabilities aware msg-oriented-protocols in#196, caps sec nods inI added a "wants to have" method to `Context` showing how I think wecould offer a pretty neat msg-type-set-as-capability-for-protocolsystem.
goodboy added a commit that referenced this pull requestMar 27, 2025
goodboy added a commit that referenced this pull requestMar 27, 2025
As per the long outstanding GH issue this starts our rigorous journeyinto an attempt at a type-safe, cross-actor SC, IPC protocol Boboop ->#36The idea is to "formally" define our SC "shuttle (dialog) protocol" byspecifying a new `.msg.types.Msg` subtype-set which can fullyencapsulate all IPC msg schemas needed in order to accomplishcross-process SC!The msg set deviated a little in terms of (type) names from the existing`dict`-msgs currently used in the runtime impl but, I think the namechanges are much better in terms of explicitly representing the internalsemantics of the actor runtime machinery/subsystems and theIPC-msg-dialog required for SC enforced RPC.------ - ------In cursory, the new formal msgs-spec includes the following msg-subtypesof a new top-level `Msg` boxing type (that holds the base field schemafor all msgs):- `Start` to request RPC task scheduling by passing a `FuncSpec` payload (to replace the currently used `{'cmd': ... }` dict msg impl)- `StartAck` to allow the RPC task callee-side to report a `IpcCtxSpec` payload immediately back to the caller (currently responded naively via a `{'functype': ... }` msg)- `Started` to deliver the first value from `Context.started()` (instead of the existing `{'started': ... }`)- `Yield` to shuttle `MsgStream.send()`-ed values (instead of our `{'yield': ... }`)- `Stop` to terminate a `Context.open_stream()` session/block (over `{'stop': True }`)- `Return` to deliver the final value from the `Actor.start_remote_task()` (which is a `{'return': ... }`)- `Error` to box `RemoteActorError` exceptions via a `.pld: ErrorData` payload, planned to replace/extend the current `RemoteActorError.msgdata` mechanism internal to `._exceptions.pack/unpack_error()`The new `tractor.msg.types` includes all the above msg defs as well an APIfor rendering a "payload type specification" using a`payload_type_spec: Union[Type]` that can be passed to`msgspec.msgpack.Decoder(type=payload_type_spec)`. This ensures that(for a subset of the above msg set) `Msg.pld: PayloadT` data istype-parameterized using `msgspec`'s new `Generic[PayloadT]` fieldsupport and thus enables providing for an API where IPC `Context`dialogs can strictly define the allowed payload-datatype-set via typeunion!Iow, this is the foundation for supporting `Channel`/`Context`/`MsgStream`IPC primitives which are type checked/safe as desired in GH issue:-#365Misc notes on current impl(s) status:------ - ------- add a `.msg.types.mk_msg_spec()` which uses the new `msgspec` support for `class MyStruct[Struct, Generic[T]]` parameterize-able fields and delivers our boxing SC-msg-(sub)set with the desired `payload_types` applied to `.pld`: -https://jcristharif.com/msgspec/supported-types.html#generic-types - as a note this impl seems to need to use `type.new_class()` dynamic subtype generation, though i don't really get *why* still.. but without that the `msgspec.msgpack.Decoder` doesn't seem to reject `.pld` limited `Msg` subtypes as demonstrated in the new test.- around this ^ add a `.msg._codec.limit_msg_spec()` cm which exposes this payload type limiting API such that it can be applied per task via a `MsgCodec` in app code.- the orig approach in#311 was the idea of making payload fields `.pld: Raw` wherein we could have per-field/sub-msg decoders dynamically loaded depending on the particular application-layer schema in use. I don't want to lose the idea of this since I think it might be useful for an idea I have about capability-based-fields(-sharing, maybe using field-subset encryption?), and as such i've kept the (ostensibly) working impls in TODO-comments in `.msg._codec` wherein maybe we can add a `MsgCodec._payload_decs: dict` table for this later on. |_ also left in the `.msg.types.enc/decmsg()` impls but renamed as `enc/dec_payload()` (but reworked to not rely on the lifo codec stack tables; now removed) such that we can prolly move them to `MsgCodec` methods in the future.- add an unused `._codec.mk_tagged_union_dec()` helper which was originally factored out the#311 proto-code but didn't end up working as desired with the new parameterized generic fields approach (now in `msg.types.mk_msg_spec()`)Testing/deps work:------ - ------- new `test_limit_msgspec()` which ensures all the `.types` content is correct but without using the wrapping APIs in `._codec`; i.e. using a in-line `Decoder` instead of a `MsgCodec`.- pin us to `msgspec>=0.18.5` which has the needed generic-types support (which took me way too long yester to figure out when implementing all this XD)!
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Labels
discussionenhancementNew feature or request experimentExploratory design and testing IPC and transportmessagingmessaging patterns and protocols
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.
Uh oh!
There was an error while loading.Please reload this page.
WIP typed msg layer using
msgspec
.Relates to:
Further motivation for the approach being taken is hashed out somewhat injcrist/msgspec#140
still needs lootttsa work..