Added a 'cancellable' set of conversions for proper cancellation from Python #48
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ncelled from Python (may become default behaviour in next release)
Concur, because this library is all about "magically" transforming rust futures to asyncio futures, and back, and cancellation (or simply dropping a future in rust) is a big part of either functionality, so i'd say that making this the default is reasonable and expected. |
|
One thing; if you're going to instantly deprecate this in 0.15 and make it the default, then also note that in the documentation. |
|
Yeah I agree, that's a good point. |
added 7 commits
September 26, 2021 07:41
…into better-cancellation
… 'clap', is now locked at MSRV 1.46
…buntu to continue
…times and added new tests for them
|
Finally wrapping this PR up!
Should be ready for a 0.14.1 release unless something comes up last minute! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses the issue brought up in #47 where the Rust future keeps running after the
asyncio.Futurehas been cancelled.For now, I've added this functionality as a separate set of
cancellable_future_into_py*conversions, but I think this might be better served as the default functionality rather than an opt-in functionality. I'm open to feedback and suggestions on this though!As it is now, it can be released as a patch, but in 0.15 the functionality in
cancellable_future_into_py*might just be baked-in by default and thecancellable_future_into_py*conversions would be deprecated.