Open graingert opened 2 months ago
Sure, why not 🙂
Although I'll note that even if you correctly use with aclosing(...) as ait:
in every single scope, you still have to avoid ever yield
ing across a cancel scope.
I see two different ways of implementing this:
AsyncIterator
or AsyncIterable
is used as return type.yield
inside (except if that's a nested function), and give an error if there is a return type and it is not AsyncGenerator
.1 is significantly simpler to implement, avoids false positives, and sounds like it resolves the issue just as well? Whereas the title of the issue implies 2.
(1) fails for functions which return a ReceiveChannel
, though.
I'd treat this as a distinctly low priority issue since it's nontrivial to implement and I prefer to ban async generators anyway, but no objection if someone wants to contribute it.
This should probably not trigger for functions decorated with @pytest.fixture
as AsyncIterator
is just fine there.
Not if you're using pytest-trio - the yield can raise trio.Cancelled https://pytest-trio.readthedocs.io/en/stable/reference.html#an-important-note-about-yield-fixtures
Now, here’s the punchline: this means that in our examples above, the teardown code might not be executed at all! This is different from how pytest fixtures normally work. Normally, the yield in a pytest fixture never raises an exception, so you can be certain that any code you put after it will execute as normal. But if you have a fixture with background tasks, and they crash, then your yield might raise an exception, and Python will skip executing the code after the yield.
This means the fixture function needs to return something with .athrow()
That sounds like a severe bug in pytest-trio.
No, that seems fine to me? The docs are a bit odd, but this is just like applying @asynccontextmanager
.
Typing an async generator as AsyncIterator or AsyncIterable prevents wrapping it with contextlib.aclosing