encode
Ensure that all async generators are explicitly closed
Summary
This change avoids warnings on Trio by explicitly closing all async generators rather than relying on the garbage collector to do so, as this may cause unpredictable behavior due to different GC implementations.
Checklist
- [X] I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
- [X] I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
- [X] I've updated the documentation accordingly.
There are no new tests but I've removed the xfail markers for the existing cancellation tests.
Would it be okay to upgrade Mypy? Later versions are fine with the single type parameter to AsyncGenerator.
Would it be okay to upgrade Mypy?
Yep.
You're welcome to take the shortcut and update that here, or else issue a separate PR dealing just with that.
What's the policy on Python support btw? Do you want to keep supporting the EOL'd 3.8?
Okay with dropping support for EOL'd Pythons.
Done. I also updated the setup-python action and added Python 3.13 to the CI. This currently makes its CI job take longer than the other Python versions, due to the facts that 1) the workflow doesn't use a pip cache, 2) it installs a lot of heavy dependencies which have nothing to do with running the test suite and 3) it pins matplotlib to an old version that does not have Python 3.13 wheels. I can fix any of the above issues, just let me know what you'd like me to do.
Note that I had to add two # pragma: no cover markers as otherwise coverage would drop due to diverging code paths based on the Python version. Let me know if you'd like me to switch the CI to use coverage combining instead.
I found a couple more spots where async iterables that may be generators aren't closed.
@Kludex I replaced the awkward AsyncExitStack mess with a much cleaner solution which I also used in my httpx PR (https://github.com/encode/httpx/pull/3593). Sorry for the extra churn!
@tomchristie This PR and the one in httpx are pre-requisites to a PR @agronholm is working in here: https://github.com/modelcontextprotocol/python-sdk/pull/946 (I'm helping on that project).
I know I have merge rights here, but I don't feel comfortable merging here. 👀
Would you mind checking if something is needed on those PRs to move forward? 🙏
Hi @agronholm @Kludex just coming from https://github.com/modelcontextprotocol/python-sdk/pull/946 where I think this is the last dependency update needed.
It looks like this PR is approved and might just need resolution of some merge conflicts? Keen to understand if we need anything else to get this landed :)
