Skip to content

Let the Nursery context-manage itself - #340

Closed
m-u-xyz wants to merge 3 commits into
python-trio:masterfrom
smurfix:nursery
Closed

m-u-xyz wants to merge 3 commits into
python-trio:masterfrom
smurfix:nursery

Conversation

@m-u-xyz

@m-u-xyz m-u-xyz commented Oct 19, 2017

Copy link
Copy Markdown
Contributor

I was curious whether the commented-out NurseryManager class would actually work, so I turned it on ;-)
I had to insert one conditional because of an impedance mismatch between context managing and cancel handling. All tests pass.

On second thought (second patch), a Nursery can be its own context manager. This can be improved further (third patch) by deferring the initialization to __aenter__(), noting that a Nursery is never used outside of a context manager.

This change speeds up the full test run by a bit more than 1% on my system (tested with 3.5 and 3.6).

@njsmith

njsmith commented Nov 28, 2017

Copy link
Copy Markdown
Member

I should note here: the nursery implementation is going to get majorly revisited in 0.3.0 (#136), so I've been putting this off to look at as part of that.

@njsmith

njsmith commented Dec 19, 2017

Copy link
Copy Markdown
Member

First pass of simplified nurseries is in #375.

Do you remember why you had to add the if isinstance(exc, Cancelled): return True bit to your Nursery.__aexit__ method? I don't see how it can be correct (doesn't it make nurseries swallow all Cancelled exceptions?), but I assume you ran into trouble without it...

@m-u-xyz

m-u-xyz commented Dec 20, 2017

Copy link
Copy Markdown
Contributor Author

I'll test my code with the current master branch and report back.

@njsmith

njsmith commented Dec 20, 2017

Copy link
Copy Markdown
Member

Thanks!

@kyrias

kyrias commented Feb 1, 2018

Copy link
Copy Markdown
Contributor

Any updates on this? :)

@m-u-xyz

m-u-xyz commented Feb 1, 2018

Copy link
Copy Markdown
Contributor Author

I'll have a look soon(ish).

@njsmith

njsmith commented Apr 22, 2018

Copy link
Copy Markdown
Member

We should probably stop using @asynccontextmanager in the nursery implementation (#229, #393). But this branch is way out of date, discussion has stalled, and I think it goes a bit to far (I like that the actual Nursery object is only returned from __aenter__, not from open_nursery). I'm going to close this for now, and we can always re-open it or steal bits from it later if we want to.

@njsmith njsmith closed this Apr 22, 2018
@belm0 belm0 mentioned this pull request Aug 18, 2018
3 of 11 tasks
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.

3 participants