-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Attempted fix for "unexpected cancel" issue in #34 #35
Conversation
7e41d52
to
afcfc04
Compare
Codecov Report
@@ Coverage Diff @@
## main #35 +/- ##
==========================================
- Coverage 78.74% 78.63% -0.11%
==========================================
Files 11 11
Lines 941 941
==========================================
- Hits 741 740 -1
- Misses 200 201 +1
Continue to review full report at Codecov.
|
afcfc04
to
07ed0fb
Compare
@achimnol Here's another one, though I'd appreciate a careful look at my solution to ensure I haven't broken something unintentionally (though all tests pass!) |
I think this would be better reviewed by the original authors of the module, @1st1 and @elprans. Note that Python 3.11 will introduce the concept of |
That does sound really nice (was unaware of that PEP), though because a lot of us will be stuck on older Pythons for quite a while (Lambda is a good example, it's still on 3.9 unless you roll your own runtime) having a compatibility mode would be great. |
I'm not sure yet how Python 3.11 will organize the TaskGroup API in the stdlib asyncio (which is the main motivation of ExceptionGroup), but if it turns out good, we could leave the current aiotools' implementation as an alternative option for old Python users. At that time, we could bump the version of aiotools to 2.0 and drop backward-compatilibility to Python 3.6 and 3.7, as 3.6 is already EOL and 3.7 would become EOL in June the next year, while 3.11 is expected to be released on October this year. |
Also take a look at python/cpython#31270! |
That's awesome, I've lost count of how many times I've shot myself in the foot due to dangling tasks. Having this in stdlib should help a lot. |
Any word on this? This is actually somewhat of an issue for an application I'm trying to use task groups for. I can of course just copy an implementation into my code, but an upstream fix would be nicer. :) |
* Take the implementation of `_is_base_error()` from Python 3.11's `asyncio.TaskGroup` instead of changing `__aexit__()` method. * Use more specific exception class in the test case to prevent unintended catch-all situation in terms of regression check.
I think the problem is |
👍 LGTM I can confirm that just the |
This attempts to fix #34 . I'm not quite sure what the intended logic behind the
base_error
stuff was, and as such I'm not sure if I've messed something up here. But the tests pass! :)