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

Heh, yeah I guess tractor.run() is kinda dumb.. #177

Closed
Tracked by #197
goodboy opened this issue Dec 23, 2020 · 5 comments
Closed
Tracked by #197

Heh, yeah I guess tractor.run() is kinda dumb.. #177

goodboy opened this issue Dec 23, 2020 · 5 comments
Labels
api discussion help wanted Extra attention is needed

Comments

@goodboy
Copy link
Owner

goodboy commented Dec 23, 2020

This is somewhat of a 🤦🏼 and directly related to #168.

Thanks to the trio crowd for pointing out that we can probably just make ourselves a:

import tractor
import trio

async main():
    async with tractor.open_runtime():
        .. do tractor things since this is a task...
        
trio.run(main)

Some benefits to this:

  • it's not scary since it's trio.run() 🤯
  • we can run certain parts of code with debug_mode=True ?
  • the whole process tree can be restarted easily from scratch
  • we aren't hijacking trio.run() when we really have no reason to

Some worries:

  • starting multiple "runtimes" (aka root actors) in a single program will cause mailbox collisions
    • maybe we could have a .open_root_runtime() which can only be called once per program?
    • is this solved by just avoiding channel server address collisions?
  • maybe more?

Is there a better name?

  • .open_root()
  • .start_runtime()
  • start_root()

Lurkerzzzz!!!

@goodboy goodboy added help wanted Extra attention is needed discussion api labels Dec 23, 2020
@richardsheridan
Copy link

IMHO the ideal API would be:

async with tractor.open_actor_nursery() as n:
    n.start_soon(async_fn)

And tractor should take care of any global bookkeeping internally, if there is such a thing. $0.02 :)

@goodboy
Copy link
Owner Author

goodboy commented Dec 23, 2020

@richardsheridan only problem is there might only ever be one "actor_nursery* open per program.

Also note, in current parlance, an ActorNursery is a nursery which spawns actors - aka new processes running trio.run(). So I don't think calling the top level entry point for starting all the machinery/runtime (that makes actor spawning possible) a "nursery" necessarily makes sense. It's actually more along the lines of the root task in trio, I think.

maybe .open_root_actor()?

@goodboy
Copy link
Owner Author

goodboy commented Dec 24, 2020

Hmm thinking more on this I wonder if we can just do implicit runtime startup (only in the root process) on the first entry to tractor.open_nursery()?

I'm thinking there should still be an explicit .start_root_actor_runtime() or whatever but that can be implicitly called if we find tractor.is_root_process() == True when entering the .open_nursery() block.

goodboy added a commit that referenced this issue Dec 27, 2020
This begins the move to dropping support for `tractor.run()` which we
don't really need since the runtime is started (as it always has been)
from a new sub-task / nursery. Instead this introduces starting the
actor tree through a `open_root_actor()` async context manager which
we'll likely implicitly call (from the root) on the first use of an
actor nursery.

Drop `_actor._start_actor()` and factor its contents into this new api.
Make `run()` and `run_daemon()` use `open_root_actor()` until we decide
to remove them.

Relates to #168 and #177
@goodboy
Copy link
Owner Author

goodboy commented Dec 27, 2020

The removal of tractor.run() is going to require a big edit of the test set and examples.

goodboy added a commit that referenced this issue Dec 27, 2020
This begins the move to dropping support for `tractor.run()` which we
don't really need since the runtime is started (as it always has been)
from a new sub-task / nursery. Instead this introduces starting the
actor tree through a `open_root_actor()` async context manager which
we'll likely implicitly call (from the root) on the first use of an
actor nursery.

Drop `_actor._start_actor()` and factor its contents into this new api.
Make `run()` and `run_daemon()` use `open_root_actor()` until we decide
to remove them.

Relates to #168 and #177
goodboy added a commit that referenced this issue Dec 27, 2020
This begins the move to dropping support for `tractor.run()` which we
don't really need since the runtime is started (as it always has been)
from a new sub-task / nursery. Instead this introduces starting the
actor tree through a `open_root_actor()` async context manager which
we'll likely implicitly call (from the root) on the first use of an
actor nursery.

Drop `_actor._start_actor()` and factor its contents into this new api.
Make `run()` and `run_daemon()` use `open_root_actor()` until we decide
to remove them.

Relates to #168 and #177
@goodboy goodboy mentioned this issue Feb 24, 2021
8 tasks
@goodboy
Copy link
Owner Author

goodboy commented Jul 31, 2021

Resolved with #197.
Full .run() removal will come in a upcoming release.

@goodboy goodboy closed this as completed Jul 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api discussion help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants