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

Drop the "main" task via kwarg idea #25

Merged
merged 6 commits into from
Aug 4, 2018
Merged

Drop the "main" task via kwarg idea #25

merged 6 commits into from
Aug 4, 2018

Conversation

goodboy
Copy link
Owner

@goodboy goodboy commented Aug 2, 2018

Stop worrying about a "main task" in each actor and instead add an
additional ActorNursery.run_in_actor() method which wraps calls
to create an actor and run a lone RPC task inside it. Note this
adjusts the public API of ActorNursery.start_actor() to drop
its main kwarg.

The dirty deats of making this possible:

  • each spawned RPC task is now tracked with a specific cancel scope such
    that when the actor is cancelled all ongoing responders are cancelled
    before any IPC/channel machinery is closed (turns out that spawning
    new actors from outlive_main=True actors was probably borked before
    finally getting this working).
  • make each initial RPC response be a packet which describes the
    functype (eg. {'functype': 'asyncfunction'}) allowing for async
    calls/submissions by client actors (this was required to make
    run_in_actor() work - Portal._submit() is the new async method).
  • hooray we can stop faking "main task" results for daemon actors
  • add better handling/raising of internal errors caught in the bowels of
    the Actor itself.
  • drop the rpc spawning nursery; just use the Actor._root_nursery
  • only wait on _no_more_peers if there are existing peer channels that
    are actually still connected.
  • an ActorNursery.__aexit__() now implicitly waits on Portal.result() on close
    for each run_in_actor() spawned actor.
  • handle cancelling partial started actors which haven't yet connected
    back to the parent

Resolves #24

This will require updates to #11 obviously.

ping @vodik!

Tyler Goodlet added 4 commits August 2, 2018 15:24
Stop worrying about a "main task" in each actor and instead add an
additional `ActorNursery.run_in_actor()` method which wraps calls
to create an actor and run a lone RPC task inside it. Note this
adjusts the public API of `ActorNursery.start_actor()` to drop
its `main` kwarg.

The dirty deats of making this possible:
- each spawned RPC task is now tracked with a specific cancel scope such
  that when the actor is cancelled all ongoing responders are cancelled
  before any IPC/channel machinery is closed (turns out that spawning
  new actors from `outlive_main=True` actors was probably borked before
  finally getting this working).
- make each initial RPC response be a packet which describes the
  `functype` (eg. `{'functype': 'asyncfunction'}`) allowing for async
  calls/submissions by client actors (this was required to make
  `run_in_actor()` work - `Portal._submit()` is the new async method).
- hooray we can stop faking "main task" results for daemon actors
- add better handling/raising of internal errors caught in the bowels of
  the `Actor` itself.
- drop the rpc spawning nursery; just use the `Actor._root_nursery`
- only wait on `_no_more_peers` if there are existing peer channels that
  are actually still connected.
- an `ActorNursery.__aexit__()` now implicitly waits on `Portal.result()` on close
  for each `run_in_actor()` spawned actor.
- handle cancelling partial started actors which haven't yet connected
  back to the parent

Resolves #24
goodboy pushed a commit that referenced this pull request Aug 3, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
goodboy pushed a commit that referenced this pull request Aug 3, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
goodboy pushed a commit that referenced this pull request Aug 3, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
if functype == 'function' or functype == 'asyncfunction':
resp_type = 'return'
elif functype == 'asyncgen':
resp_type = 'yield'
Copy link

Choose a reason for hiding this comment

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

Have you considered using an enum here instead of strings?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Added #27 for this.

log.debug(
f"Cancelling all tasks for {chan} from {chan.uid}")
scopes = self._rpc_tasks.pop(chan, None)
if scopes:
Copy link

Choose a reason for hiding this comment

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

If you do scopes = self._rpc_tasks.pop(chan, ()) instead, you can skip the if.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Good point.

@goodboy goodboy merged commit 140956a into master Aug 4, 2018
goodboy pushed a commit that referenced this pull request Aug 4, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
goodboy pushed a commit that referenced this pull request Aug 6, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
goodboy pushed a commit that referenced this pull request Aug 6, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
goodboy pushed a commit that referenced this pull request Aug 6, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
goodboy pushed a commit that referenced this pull request Aug 6, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
goodboy pushed a commit that referenced this pull request Aug 6, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
goodboy pushed a commit that referenced this pull request Aug 6, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
goodboy pushed a commit that referenced this pull request Aug 6, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
goodboy pushed a commit that referenced this pull request Aug 6, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
goodboy pushed a commit that referenced this pull request Aug 6, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
goodboy pushed a commit that referenced this pull request Aug 6, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
goodboy pushed a commit that referenced this pull request Aug 6, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
goodboy pushed a commit that referenced this pull request Aug 6, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
goodboy pushed a commit that referenced this pull request Aug 6, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
goodboy pushed a commit that referenced this pull request Aug 7, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
goodboy pushed a commit that referenced this pull request Aug 7, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
goodboy pushed a commit that referenced this pull request Aug 7, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
goodboy pushed a commit that referenced this pull request Aug 7, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
goodboy pushed a commit that referenced this pull request Aug 7, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
goodboy pushed a commit that referenced this pull request Aug 7, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
goodboy pushed a commit that referenced this pull request Aug 7, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
goodboy pushed a commit that referenced this pull request Aug 7, 2018
Revamp the docs after some feedback from @vodik.
See #24 #25 for additional details.
@goodboy goodboy deleted the drop_main_kwarg branch July 21, 2020 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The main task idea is really really wrong!
2 participants