[docs] clarify checkpoint semantics for trio.open_nursery #3011
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3011 +/- ##
=======================================
Coverage 99.63% 99.63%
=======================================
Files 120 120
Lines 17855 17857 +2
Branches 3212 3212
=======================================
+ Hits 17790 17792 +2
Misses 46 46
Partials 19 19
|
| @@ -102,6 +102,8 @@ them. Here are the rules: | |||
| only that one will act as a checkpoint. This is documented | |||
| on a case-by-case basis. | |||
|
|
|||
| * :func:`trio.open_nursery` is a further exception to this rule. | |||
There was a problem hiding this comment.
Not sure if this is needed, I think the Partial exception for async context managers block handles this
There was a problem hiding this comment.
It may be overkill, but open_nursery in itself as a context manager doesn't guarantee a full checkpoint on either of entry or exit - so imo it doesn't abide by the special rules in the Partial exception for async context managers block. There's some more discussion around this in #1457
src/trio/_core/_run.py
Outdated
| @@ -993,7 +993,8 @@ def open_nursery( | |||
| new `Nursery`. | |||
| It does not block on entry; on exit it blocks until all child tasks | |||
| have exited. | |||
| have exited. If no child tasks are running on exit, it will insert a | |||
| schedule point (but no cancellation point). | |||
There was a problem hiding this comment.
Since this is unique behaviour, maybe it'd be good to add an additional sentence explaining why this is the case - something like "A nursery is never the source of the cancellation exception, it only propagates it from sub-tasks."
There was a problem hiding this comment.
Oh actually, is that not functionally the same as https://trio.readthedocs.io/en/stable/reference-lowlevel.html#trio.lowlevel.cancel_shielded_checkpoint ?
If so, we can refer to that for a more thorough explanation
There was a problem hiding this comment.
I don't think just referring to that builds any kind of intuition for why open_nursery is a special case. In fact I forgot the exact wording/problem that convinced me, but I think it was some message on the gitter (that pushed the old PR to be merged).
There was a problem hiding this comment.
Pushed a small commit that both refers to cancel_shielded_checkpoint and includes teamspen's blurb on what this implies. Not sure if that also covers @A5rocks comment?
There was a problem hiding this comment.
Not exactly, but I think docs as is are good enough:tm:
Documents the changes made in #1696 more explicitly.