feat: handle CancelledError - cancel if no other waiters + refactoring#708
feat: handle CancelledError - cancel if no other waiters + refactoring#708BobTheBuidler wants to merge 29 commits intoaio-libs:masterfrom
Conversation
CodSpeed Performance ReportMerging #708 will improve performances by ×560Comparing Summary
Benchmarks breakdown
|
|
Yeah this is better than #697 , check out the performance gains that come with it |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #708 +/- ##
==========================================
- Coverage 97.22% 97.05% -0.17%
==========================================
Files 12 13 +1
Lines 793 816 +23
Branches 41 42 +1
==========================================
+ Hits 771 792 +21
- Misses 20 22 +2
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @property | ||
| def __tasks(self) -> List["asyncio.Task[_R]"]: | ||
| # NOTE: I don't think we need to form a set first here but not too sure we want it for guarantees | ||
| return list( |
There was a problem hiding this comment.
I think a list is probably overkill, we could just return an iterator for all the uses I saw.
| { | ||
| cache_item.task | ||
| for cache_item in self.__cache.values() | ||
| if not cache_item.task.done() |
There was a problem hiding this comment.
I don't think the condition is needed or useful for the use cases I saw.
|
I think we can merge the appropriate changes into the other PR, to keep reviewing in one place. |
| self.__closed = True | ||
|
|
||
| tasks = list(self.__tasks) | ||
| tasks = self.__tasks |
There was a problem hiding this comment.
we need to materialize tasks into a list or set or some other container here
There was a problem hiding this comment.
Well, that's already handled by the original code.
2 participants
This is a second option for #697 with the comments in that PR implemented here for separate review