-
Notifications
You must be signed in to change notification settings - Fork 12
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
Initial readme documenting most features #11
Conversation
ec67457
to
9af8040
Compare
tractor/__init__.py
Outdated
await chan.send({'stop': None, 'cid': cid}) | ||
with trio.open_cancel_scope() as cs: | ||
async for item in coro: | ||
# TODO: can we send values back in here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vodik interestingly enough I had a bug (just below) in this section but this comment is relevant to some of the stuff we talked about. My main outstanding concern here is that I'm not sure how to non-blocking read from the underlying socket if nothing has been asend()
from the other side (assuming we don't send an actual None
through on every iteration). I know you said maybe that's how it should work but it still seems like unnecessary overhead to me..
c251ded
to
036c7c8
Compare
@vodik, @dnephin, @nonsleepr gonna merge this soon if no-one is going to comment - no big deal if you never found the time. |
README.rst
Outdated
Transparent function calling using *portals* | ||
-------------------------------------------- | ||
``tractor`` introdces the concept of a *portal* which is an API | ||
borrowed_ from ``trio``. A portal may seems similar to the idea of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo, should be A portal may seem similar
README.rst
Outdated
|
||
Actors communicate with each other by sending *messages* over channels_, but the details of this | ||
in ``tractor`` is by default hidden and *actors* can instead easily invoke remote asynchronous | ||
functions using *portals*. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, this is a semantic game, but arguable function calls in python are messages when you consider how attribute access works in Python. Its not really all that hidden.
You could clean this up by saying "sending messages over channels to easily invoke remote ...."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, I wanted to change this sentence anyway.
README.rst
Outdated
- verbatim support for ``trio``'s cancellation_ system | ||
- no use of *proxy* objects to wrap RPC calls | ||
- an immersive debugging experience | ||
- be simple, be small |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't bother with this last point, its a sematics game. Small or large are preceptions based on perceived feature breadth and depth and fitness to the problem at hand.
I think its more interesting to instead really define your scope and list antigoals. These are things that help keep focus and a well focused code base will never feel large and boated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Swap with first class pypy3.6
support (once it comes out of course)?
README.rst
Outdated
|
||
.. warning:: ``tractor`` is in alpha-alpha and is expected to change rapidly! | ||
Expect nothing to be set in stone and your ideas about where it should go | ||
to be greatly appreciated! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would read punchier if you put a period between "stone" and "your".
|
||
What's this? Spawning event loops in subprocesses? | ||
-------------------------------------------------- | ||
Close, but not quite. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like what this header is trying to do, and I think it really works if it came immediately after the main title, but I think it falls flat since you've rambled long enough about other topics.
Might want to put a transition before this title to get back to the topic of design, or just rethink the phrasing.
I don't have any good suggestions, just that its feels like its trying to be lighthearted but came off overwhelmingly.
README.rst
Outdated
|
||
``tractor`` takes much inspiration from pulsar_ and execnet_ but attempts to be much more | ||
minimal, focus on sophistication of the lower level distributed architecture, | ||
and of course does **not** use ``asyncio``, hence **no** event loops. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rewrite this.
- This is a word salad (did you forget something): "... but attempts to be much more minimal, focus on sophistication of the ..."
- I'd suggest using "focus" instead of "minimal" - as minimal is subjective and "sophistication of the lower level distributed architecture" does not invoke ideas of minimalism - the opposite, in fact (not that there's anything wrong with not being minimal)
- Different idea, use a different sentence: "and of course does not use
asyncio
, hence no event loops"
README.rst
Outdated
this case our "director"). | ||
|
||
To gain more insight as to how ``tractor`` accomplishes all this please | ||
read on! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just drop this.
README.rst
Outdated
Actor spawning and causality | ||
---------------------------- | ||
``tractor`` tries to take ``trio``'s concept of causal task lifetimes | ||
to multi-process land. Accordingly ``tractor``'s actor nursery behaves |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comma after accordingly
README.rst
Outdated
---------------------------- | ||
``tractor`` tries to take ``trio``'s concept of causal task lifetimes | ||
to multi-process land. Accordingly ``tractor``'s actor nursery behaves | ||
similar to the nursery_ in ``trio``. That is, an ``ActorNursery`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to trio's nursery.
README.rst
Outdated
``tractor`` tries to take ``trio``'s concept of causal task lifetimes | ||
to multi-process land. Accordingly ``tractor``'s actor nursery behaves | ||
similar to the nursery_ in ``trio``. That is, an ``ActorNursery`` | ||
created with ``tractor.open_nursery()`` waits on spawned sub-actors to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Invert this sentence: "tractor.open_nursery creates ActorNursery". It moves to active voice rather than passive voice, making the chain of actions clearer.
You've also just introduced that you're talking about the trio nurseries here, so also don't blindly jump to ActorNursery - transition to it instead.
README.rst
Outdated
- the actor terminates when its *main* task completes (the default if | ||
the ``main`` kwarg is provided) | ||
- the actor can be told to ``outlive_main=True`` and thus act like an RPC | ||
daemon where it runs indefinitely until cancelled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, this is something to talk about in person, but from this reading, I see dubious value to outlive_main
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or, you're just assuming the reader will understand why its useful.
You've just brought it up casually while talking about benefit cancel scopes when it seems to subvert it. Its also then even weirder that you lead with that particular example as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a4f12c8
to
f127e2d
Compare
@vodik changes added! |
5130ed3
to
f867e9d
Compare
8d5a72d
to
14194fc
Compare
There is a slew of tests to match to verify everything documented thus far. Hopefully it's a decent little start :)
As per a suggestion from @njsmith I've added a little less verbose intro which mentions *actors* up front. More, - add install cmd - reorg spawning and portal sections a wee bit
d8a0be6
to
568dfae
Compare
There is a slew of tests to match and verify everything documented thus far.
Hopefully it's a decent little start :)
pinging @vodik, @dnephin, @nonsleepr and @Konstantine00 since you all expressed interest (or gave me permission to bug you) to take a look at this.
Please feel free to be honest and grill what you don't like 👍
Rendered version is here.
Thanks!