Skip to content
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

Add a way to shield a stream's underlying channel #171

Merged
merged 4 commits into from
Dec 18, 2020

Conversation

goodboy
Copy link
Owner

@goodboy goodboy commented Dec 17, 2020

Add a tractor._portal.StreamReceiveChannel.shield_channel() context manager which allows for avoiding the closing of an IPC stream's underlying channel for the purposes of task re-spawning.

Sometimes you might want to cancel a task consuming a stream but not tear down the IPC
between actors (the default). A common use can might be where the task's "setup" work might need to be redone but you want to keep the established portal / channel in tact despite the task restart.

Includes a test.

I'm still not sure on the name here..

Any opinions @salotz , @guilledk?

Note it's used inside a with stream.<meth name>

Short list:

  • .shield_channel()
  • .shielded_channel()
  • .shield()
  • just make a bool property .shield you assign manually?
  • .shield_from_close()
  • .channel_kept_open()
  • .channel_shielded()

Thoughts appreciated.

Add a ``tractor._portal.StreamReceiveChannel.shield_channel()`` context
manager which allows for avoiding the closing of an IPC stream's
underlying channel for the purposes of task re-spawning. Sometimes you
might want to cancel a task consuming a stream but not tear down the IPC
between actors (the default). A common use can might be where the task's
"setup" work might need to be redone but you want to keep the
established portal / channel in tact despite the task restart.

Includes a test.
@goodboy goodboy added enhancement New feature or request help wanted Extra attention is needed labels Dec 17, 2020
@contextmanager
def shield_channel(
self
) -> typing.AsyncGenerator['StreamReceiveChannel', None]:
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lol this type sig is obv wrong; originally made it async, for no good reason..


async with tractor.open_nursery() as n:

stream = await(await n.run_in_actor(
Copy link
Owner Author

@goodboy goodboy Dec 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, what do y'all think about a ActorNursery.stream_from_actor() sugar for this pattern?

The portal is feeling a little awkward (too future-y) in the streaming case, but maybe it's just me?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory you can actually just call Portal.result() before returning from .run_in_actor() but it means each call is then blocking and you'd have to spawn multiple (one-off) actors using tasks. Not sure if this is a problem and maybe will get hashed out with the discussion in #66.

@guilledk
Copy link
Collaborator

I prefer one of these:

  • .shield_channel()
  • .shielded_channel()
  • .shield()

@goodboy
Copy link
Owner Author

goodboy commented Dec 17, 2020

Sticking with .shield() for now; can always change it later.

@goodboy goodboy requested review from guilledk and salotz December 17, 2020 19:39
@goodboy
Copy link
Owner Author

goodboy commented Dec 17, 2020

Ok think this is ready finally (after wrangling some tests code along the way).

I'll probably merge in the morning if noone pipes up.

@goodboy goodboy merged commit 1701493 into master Dec 18, 2020
@goodboy goodboy deleted the stream_channel_shield branch December 18, 2020 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants