Attempted fix for "unexpected cancel" issue in #34

[bdowning]

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! :)

[codecov[bot]]

Codecov Report

Merging #35 (1db75c9) into main (02be382) will decrease coverage by 0.10%. The diff coverage is 100.00%.

@@            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     

Impacted Files	Coverage Δ
src/aiotools/taskgroup.py	91.07% <100.00%> (-0.60%)	:arrow_down:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 02be382...1db75c9. Read the comment docs.

[bdowning]

@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!)

[achimnol]

I think this would be better reviewed by the original authors of the module, @1st1 and @elprans. As you may know, our taskgroup module is imported from EdgeDB's _taskgroup module.

Note that Python 3.11 will introduce the concept of ExceptionGroup and BaseExceptionGroup (where CancelledError belongs to) with a new syntax except*: (PEP-654). aiotools currently targets Python 3.6, but I'm considering migration of taskgroup and ptaskgroup modules to Python 3.11 once it gets released, while keeping other modules (aiotools.context, ...) compatible with Python 3.6. This means that current MultiError is a kind of interim solution which needs to be replaced soon.

[bdowning]

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.

[achimnol]

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.

[achimnol]

Also take a look at python/cpython#31270!

[bdowning]

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.

[bdowning]

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. :)

[achimnol]

I think the problem is _is_base_error() implementation. Updated it to use the same logic from Python 3.11's asyncio.TaskGroup._is_base_error().

[bdowning]

:+1: LGTM

I can confirm that just the _is_base_error patch fixes the "expected cancel" errors I was seeing in my actual codebase.