Added a 'cancellable' set of conversions for proper cancellation from Python

awestlake87 / pyo3-asyncio

Other

312 stars 48 forks source link

Added a 'cancellable' set of conversions for proper cancellation from Python #48

Closed awestlake87 closed 3 years ago

awestlake87 commented 3 years ago

This PR addresses the issue brought up in #47 where the Rust future keeps running after the asyncio.Future has 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 the cancellable_future_into_py* conversions would be deprecated.

ShadowJonathan commented 3 years ago

but I think this might be better served as the default functionality rather than an opt-in functionality.

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.

ShadowJonathan commented 3 years ago

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.

awestlake87 commented 3 years ago

Yeah I agree, that's a good point.

awestlake87 commented 3 years ago

Finally wrapping this PR up!

I went ahead and added a notice to all of the new cancellable variants about the future deprecation in 0.15
Added local_cancellable_* variants to support cancellation for !Send futures
Added test for local_cancellable_* variants.

Should be ready for a 0.14.1 release unless something comes up last minute!