Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upGitHub is where the world builds software
Millions of developers and companies build, ship, and maintain their software on GitHub — the largest and most advanced development platform in the world.
WIP: Add graceful cancel to cancel scopes #941
Conversation
Codecov Report
@@ Coverage Diff @@
## master #941 +/- ##
==========================================
- Coverage 99.39% 99.03% -0.36%
==========================================
Files 102 102
Lines 12461 12249 -212
Branches 915 918 +3
==========================================
- Hits 12385 12131 -254
- Misses 56 85 +29
- Partials 20 33 +13
|
| @@ -366,8 +428,7 @@ def _exc_filter(self, exc): | |||
| def _close(self, exc): | |||
| scope_task = current_task() | |||
| if ( | |||
| exc is not None and self.cancel_called | |||
nmalaguti
Feb 19, 2019
Author
Contributor
It looks like self.cancel_called is in this if statement to ensure that raising Cancelled won't be caught here unless scope.cancel() was called. Looking at the logic in _pending_cancel_scope it looks like you can't return a scope unless that is already true, so this check is redundant.
I have removed it because there are now cases where you can be cancelled gracefully without having been explicitly cancelled.
It looks like self.cancel_called is in this if statement to ensure that raising Cancelled won't be caught here unless scope.cancel() was called. Looking at the logic in _pending_cancel_scope it looks like you can't return a scope unless that is already true, so this check is redundant.
I have removed it because there are now cases where you can be cancelled gracefully without having been explicitly cancelled.
1889f00
to
9128593
See https://trio.discourse.group/t/graceful-shutdown/93/19?u=nmalaguti for the intended behavior.
9128593
to
7ca63d5
|
@oremanj I was surprised at how simple this was to implement, and it turns out it's because you've been doing a ton of work in the cancellation space. Seeing how this relates to #886, #885, #910, and #147, I feel like I didn't do enough research before embarking on this change. Also I just noticed you have an alternate approach in #921. I'd love to discuss further the differences between them. |
| that are outside this scope that have been gracefully canceled. | ||
| If this is set to :data:`None` then this scope will inherit its | ||
| graceful cancel behavior from its parent scope. | ||
Tronic
Sep 24, 2019
Contributor
This wording is hard to understand and should be clarified.
This wording is hard to understand and should be clarified.
See https://trio.discourse.group/t/graceful-shutdown/93/19?u=nmalaguti for the intended behavior.
No tests yet, just some code to illustrate intended behavior.