On 10:18 am, te...@jon.es wrote:
Hi Kai

[snip]

2b) there is some kind of exception when
cancel calls the cancellation function.  I don't think 2a) is really an
exception situation, so it makes sense, as you say, just to return False from cancel in this case. It's basically the cancel function saying it was too late to do anything about the underlying operation but not providing
more information than that. Internally raising and catching
CancellationFailedError (as in your code) in that case seems good to me. In the case of 2b) I would just let the exception bubble up to the calling code. Agreed, it could break some existing code, but isn't that existing
code already subject to that exact failure? It's just currently
undefined/undocumented.

This seems like a bad behavior that we should fix. Allowing arbitrary exceptions from other application code here makes it a lot harder to write robust code that uses Deferred cancellation.

Beyond that, how much code has already been written that uses this feature? If it was written based on the (admittedly, meager) documentation that exists for the feature, then it won't have exception handling.

We could leave the behavior as it is and document it and require all of that code be updated and all future code be written to handle arbitrary exceptions from a cancel call. Or we could get rid of the undocumented exception case, make the existing code correct (if it was previously correct based on documented behavior) and avoid making things more difficult for all future users of Deferred cancellation.

In my experience, there are usually a lot of subtle concerns and tricky corner cases when fixing an inconsistency between documentation and implementation. This case seems like a much more clear-cut win to just fix the implementation.

Jean-Paul

_______________________________________________
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

Reply via email to