goodboy
/
tractor
1
0
Fork 2
Code Issues 4 Pull Requests 10 Packages Projects Releases Wiki Activity

Extension types support via msgspec.Encoder/Decoder hooks #19

Open
goodboy wants to merge 19 commits from ext_type_plds into py313_support
pull from: ext_type_plds
merge into: goodboy:py313_support
Conversation 0 Commits 19 Files Changed 16
goodboy commented 2025-03-25 17:04:57 +00:00
Owner

Primary new-feature summary

Namely, extension-types as msgspec.Struct fields such that you can now (more) easily define and configure dec/enc_hook() funcs to be used by our tractor.msg.MsgCodec/Dec internals:

  • c302b74008 Finally get type-extended msgspec fields workinn

    • By using our new PldRx design we can,
      • pass through the pld-spec & a dec_hook() to our MsgDec which is used to configure the underlying .dec: msgspec.msgpack.Decoder
      • pass through a enc_hook() to mk_codec() and use it to conf the equiv MsgCodec.enc such that sent msg-plds are converted prior to transport.

  • 29c3895 Namely moving enc/dec_type_union() from the test mod to a new tractor.msg._exts for use everywhere.

  • 08e15f441 Raise RTE from limit_plds() on no curr_ctx

Various (some very serious) fixes to the tractor.msg related underbelly,

  • e57ac63f56 Avoid attr-err when MsgTypeError._ipc_msg==None on sender side.

  • fde6d84 Fix msg-draining on parent_never_opened_stream!

    • this one is For-Serious good we caught it; thx @guilledk!

  • b0ab77a Slight PldRx rework to simplify

  • 01b5955 Add Context._outcome_msg use new PldRx API

  • 51f7280 Add MsgStream._stop_msg use new PldRx API

  • 4d9f6e73 Support ctx: UnionType annots for @tractor.context eps

  • ef7a585 Deliver a MaybeBoxedError from .expect_ctxc()

Testing related fixes & improvements,

  • b6e49029 reworked test suite test_ext_types_msgspec.py,
    • rename and massively simplify to a new test_ext_types_over_ipc which avoids all the wacky “parent dictates what sender should be able to send beforehand”, instead
    • keeping it simple and just always try to send the same small set of types over the wire with expect-logic to handle each case.
  • 37b8d77 Extend ctx semantics suite for streaming edge cases!
  • 37b8d77 Extend ctx semantics suite for streaming edge cases!
Primary new-feature summary --------------------------- Namely, extension-types as `msgspec.Struct` fields such that you can now (more) easily define and configure `dec/enc_hook()` funcs to be used by our `tractor.msg.MsgCodec/Dec` internals: - c302b74008 Finally get type-extended `msgspec` fields workinn - By using our new `PldRx` design we can, - pass through the pld-spec & a `dec_hook()` to our `MsgDec` which is used to configure the underlying `.dec: msgspec.msgpack.Decoder` - pass through a `enc_hook()` to `mk_codec()` and use it to conf the equiv `MsgCodec.enc` such that sent msg-plds are converted prior to transport. - 29c3895 Namely moving `enc/dec_type_union()` from the test mod to a new `tractor.msg._exts` for use everywhere. - 08e15f441 Raise RTE from `limit_plds()` on no `curr_ctx` --- Various (some very serious) fixes to the `tractor.msg` related underbelly, --------------------------- - e57ac63f56 Avoid attr-err when `MsgTypeError._ipc_msg==None` on sender side. - fde6d84 Fix msg-draining on `parent_never_opened_stream`! - this one is **For-Serious** good we caught it; thx @guilledk! - b0ab77a Slight `PldRx` rework to simplify - 01b5955 Add `Context._outcome_msg` use new `PldRx` API - 51f7280 Add `MsgStream._stop_msg` use new `PldRx` API - 4d9f6e73 Support `ctx: UnionType` annots for `@tractor.context` eps - ef7a585 Deliver a `MaybeBoxedError` from `.expect_ctxc()` --- Testing related fixes & improvements, --------------------------- - b6e49029 reworked test suite `test_ext_types_msgspec.py`, - rename and massively simplify to a new `test_ext_types_over_ipc` which avoids all the wacky "parent dictates what sender should be able to send beforehand", instead - keeping it simple and just always try to send the same small set of types over the wire with expect-logic to handle each case. - 37b8d77 Extend ctx semantics suite for streaming edge cases! - 37b8d77 Extend ctx semantics suite for streaming edge cases!
goodboy added 19 commits 2025-03-25 17:04:57 +00:00
1c4b719e16 Finally get type-extended `msgspec` fields workinn 
By using our new `PldRx` design we can,
- pass through the pld-spec & a `dec_hook()` to our `MsgDec` which is
  used to configure the underlying `.dec: msgspec.msgpack.Decoder`
- pass through a `enc_hook()` to `mk_codec()` and use it to conf the
  equiv `MsgCodec.enc` such that sent msg-plds are converted prior
  to transport.

The trick ended up being just to always union the `mk_dec()`
extension-types spec with the normaly with the `msgspec.Raw` pld-spec
such that the `dec_hook()` is only invoked for payload types tagged
by the encoder/sender side B)

A variety of impl tweaks to make it all happen as well as various
cleanups in the `.msg._codec` mod include,

- `mk_dec()` no defaul `spec` arg, better doc string, accept the new
  `ext_types` arg, doing the union of that with `msgspec.Raw`.
- proto-ed a now unused `mk_boxed_ext_struct()` which will likely get
  removed since it ended up that our `PayloadMsg` structs already cover
  the ext-type-hook requirement that the decoder is passed
  a `.type=msgspec.Struct` of some sort in order for `.dec_hook` to be
  used.
- add a `unpack_spec_types()` util fn for getting the `set[Type]` from
  from a `Union[Type]` annotation instance.
- mk the default `mk_codec(pc_pld_spec = Raw,)` since the `PldRx` design
  was already passing/overriding it and it doesn't make much sense to
  use `Any` anymore for the same reason; it will cause various `Context`
  apis to now break.
  |_ also accept a `enc_hook()` and `ext_types` which are used to maybe
     config the `.msgpack.Encoder`
- generally tweak a bunch of comments-as-docs and todos namely the ones
  that are completed after the pld-rx design was implemented.

Also,
- mask the non-functioning `'defstruct'` approach `inside
  `.msg.types.mk_msg_spec()` to prep for its removal.

Adjust the test suite (rn called `test_caps_based_msging`),
- add a new suite `test_custom_extension_types` and move and
  use the `enc/dec_nsp()` hooks to the mod level for its use.
- prolly planning to drop the `test_limit_msgspec` suite since it's
  mostly replaced by the `test_pldrx_limiting` mod's version?
- originally was tweaking a bunch in `test_codec_hooks_mod` but likely
  it will get mostly rewritten to be simpler and simply verify that
  ext-typed fields can be used over IPC `Context`s between actors (as
  originally intended for this sub-suite).
28ab3a8a65 Move `Union` serializers to new `msg.` mod 
Namely moving `enc/dec_type_union()` from the test mod to a new
`tractor.msg._exts` for general use outside the test suite.
25f6ec770b Raise RTE from `limit_plds()` on no `curr_ctx` 
Since it should only be used from within a `Portal.open_context()`
scope, make sure the caller knows that!

Also don't hide the frame in tb if the immediate function errors..
78fd2792c7 Rework IPC-using `test_caps_basesd_msging` tests 
Namely renaming and massively simplifying it to a new
`test_ext_types_over_ipc` which avoids all the wacky "parent dictates
what sender should be able to send beforehand"..

Instead keep it simple and just always try to send the same small set of
types over the wire with expect-logic to handle each case,

- use the new `dec_hook`/`ext_types` args to `mk_[co]dec()` routines for
  pld-spec ipc transport.
- always try to stream a small set of types from the child with logic to
  handle the cases expected to error.

Other,
- draft a `test_pld_limiting_usage` to check runtime raising of bad API
  usage; haven't run it yet tho.
- move `test_custom_extension_types` to top of mod so that the
  `enc/dec_nsp()` hooks can be reffed from test parametrizations.
- comment out (and maybe remove) the old routines for
  `iter_maybe_sends`, `test_limit_msgspec`, `chk_pld_type`.

XXX TODO, turns out the 2 failing cases from this suite have exposed an
an actual bug with `MsgTypeError` unpacking where the `ipc_msg=` input
is being set to `None` ?? -> see the comment at the bottom of
`._exceptions._mk_recv_mte()` which seems to describe the likely
culprit?
9497efa7f8 Facepalm, fix logic misstep on child side 
Namely that `add_hooks: bool` should be the same as on the rent side..
Also, just drop the now unused `iter_maybe_sends`.

This makes the suite entire greeeeen btw, including the new sub-suite
which i hadn't runt before Bo
df5e377f19 Avoid attr-err when `._ipc_msg==None` 
Seems this can happen in particular when we raise a `MessageTypeError`
on the sender side of a `Context`, since there isn't any msg relayed
from the other side (though i'm wondering if MTE should derive from RAE
then considering this case?).

Means `RemoteActorError.boxed_type = None` in such cases instead of
raising an attr-error for the `None.boxed_type_str`.
9008b2a0d4 Deliver a `MaybeBoxedError` from `.expect_ctxc()` 
Just like we do from the `.devx._debug.open_crash_handler()`, this
allows checking various attrs on the raised `ContextCancelled` much like
`with pytest.raises() as excinfo:`.
b13cd4f16b Extend ctx semantics suite for streaming edge cases! 
Muchas grax to @guilledk for finding the first issue which kicked of
this further scrutiny of the `tractor.Context` and `MsgStream` semantics
test suite with a strange edge case where,
- if the parent opened and immediately closed a stream while the remote
  child task started and continued (without terminating) to send msgs
  the parent's `open_context().__aexit__()` would **not block** on the
  child to complete!
=> this was seemingly due to a bug discovered inside the
  `.msg._ops.drain_to_final_msg()` stream handling case logic where we
  are NOT checking if `Context._stream` is non-`None`!

As such this,
- extends the `test_caller_closes_ctx_after_callee_opens_stream` (now
  renamed, see below) to include cases for all combinations of the child
  and parent sending before receiving on the stream as well as all
  placements of `Context.cancel()` in the parent before, around and after
  the stream open.
- uses the new `expect_ctxc()` for expecting the taskc (`trio.Task`
  cancelled)` cases.
- also extends the `test_callee_closes_ctx_after_stream_open` (also
  renamed) to include the case where the parent sends a msg before it
  receives.
=> this case has unveiled yet-another-bug where somehow the underlying
  `MsgStream._rx_chan: trio.ReceiveMemoryChannel` is allowing the
  child's `Return[None]` msg be consumed and NOT in a place where it is
  correctly set as `Context._result` resulting in the parent hanging
  forever inside `._ops.drain_to_final_msg()`..

Alongside,
- start renaming using the new "remote-task-peer-side" semantics
  throughout the test module: "caller" -> "parent", "callee" -> "child".
7f57cbfc0e Fix msg-draining on `parent_never_opened_stream`! 
Repairs a bug in `drain_to_final_msg()` where in the `Yield()` case
block we weren't guarding against the `ctx._stream is None` edge case
which should be treated a `continue`-draining (not a `break` or
attr-error!!) situation since the peer task maybe be continuing to send
`Yield` but has not yet sent an outcome msg (one of
`Return/Error/ContextCancelled`) to terminate the loop. Ensure we
explicitly warn about this case as well as `.cancel()` emit on a taskc.

Thanks again to @guille for discovering this!

Also add temporary `.info()`s around rxed `Return` msgs as part of
trying to debug a different bug discovered while updating the
context-semantics test suite (in a prior commit).
680e479b73 Complete rename to parent->child IPC ctx peers 
Now changed in all comments docs **and** test-code content such that we
aren't using the "caller"->"callee" semantics anymore.
debe63f4f2 Slight `PldRx` rework to simplify 
Namely renaming and tweaking the `MsgType` receiving methods,
- `.recv_msg()` from what was `.recv_msg_w_pld()` which both receives
  the IPC msg from the underlying `._rx_chan` and then decodes its
  payload with `.decode_pld()`; it now also log reports on the different
  "stage of SC dialog protocol" msg types via a `match/case`.
- a new `.recv_msg_nowait()` sync equivalent of ^ (*was*
  `.recv_pld_nowait()`) who's use was the source of a recently
  discovered bug where any final `Return.pld` is being
  consumed-n-discarded by by `MsgStream.aclose()` depending on
  ctx/stream teardown race conditions..

Also,
- remove all the "instance persistent" ipc-ctx attrs, specifically the
  optional `_ipc`, `_ctx` and the `.wraps_ipc()` cm, since none of them
  were ever really needed/used; all methods which require
  a `Context/MsgStream` are explicitly always passed.
- update a buncha typing namely to use the more generic-styled
  `PayloadT` over `Any` and obviously `MsgType[PayloadT]`.
c2d1bbe05e Add `Context._outcome_msg` use new `PldRx` API 
Such that any `Return` is always capture for each ctx instance and set
in `._deliver_msg()` normally; ensures we can at least introspect for it
when missing (like in a recently discovered stream teardown race bug).
Yes this augments the already existing `._result` which is dedicated for
the `._outcome_msg.pld` in the non-error case; we might want to see if
there's a nicer way to directly proxy ref to that without getting the
pre-pld-decoded `Raw` form with `msgspec`?

Also use the new `ctx._pld_rx.recv_msg()` and drop assigning
`pld_rx._ctx`.
ab0f5c0019 Add `MsgStream._stop_msg` use new `PldRx` API 
In particular ensuring we use `ctx._pld_rx.recv_msg_nowait()` from
`.receive_nowait()` (which is called from `.aclose()`) such that we
ALWAYS (can) set the surrounding `Context._result/._outcome_msg` attrs
on reception of a final `Return`!!

This fixes a final stream-teardown-race-condition-bug where prior we
normally didn't set the `Context._result/._outcome_msg` in such cases.
This is **precisely because**  `.receive_nowait()` only returns the
`pld` and when called from `.aclose()` this value is discarded, meaning
so is its boxing `Return` despite consuming it from the underlying
`._rx_chan`..

Longer term this should be solved differently by ensuring such races
cases are handled at a higher scope like inside `Context._deliver_msg()`
or the `Portal.open_context()` enter/exit blocks? Add a detailed warning
note and todos for all this around the special case block!
b85f301c05 Mask top level import of `.hilevel` 
Since it isn't required until the landing of the new service-manager
stuff in #12; was an oversight
from commit `0607a31dddeba032a2cf7d9fe605edd9d7bb4846`.
goodboy force-pushed ext_type_plds from b85f301c05 to 8d12ece8d6 2025-03-25 20:01:05 +00:00 Compare
This pull request can be merged automatically.
You are not authorized to merge this pull request.
You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.
git checkout -b ext_type_plds py313_support
git pull origin ext_type_plds

Step 2:

Merge the changes and update on Gitea.
git checkout py313_support
git merge --no-ff ext_type_plds
git push origin py313_support
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: goodboy/tractor#19
There is no content yet.
Delete Branch "ext_type_plds"

Deleting a branch is permanent. Although the deleted branch may exist for a short time before cleaning up, in most cases it CANNOT be undone. Continue?