bpo-32363: Disable Task.set_exception() and Task.set_result()#4923
Conversation
There was a problem hiding this comment.
It's backward incompatible change.
I know people use task.set_exception() in production code, heard about it on conference couple months ago.
I propose raising a deprecation warning in 3.7 and explicit exception (I thought aboutRuntimeError but NotImplementedeError is also good maybe) in 3.8.
There was a problem hiding this comment.
But it's impossible to use it, i've shown that in the issue. That code is almost certainly broken. Maybe it's OK to force people to fix nonsense code?
There was a problem hiding this comment.
Let's pause for day or two: I'm going to wrote email to that guy for clarification.
There was a problem hiding this comment.
Hi! Thats correct, we used it on prod for monitoring purposes. However as said above it was broken, thats what I said on conference. When you try to inherit from Task on Py>3.6 (on 3.5 it is ok) that is not obvious, but you inherit CTask which not uses this method.
However, I guess why method raises "Not implemented" and called with magic super just bellow declaration. It looks strange. I see two solutions that looks better:
- Just remove it (may be also make C implementation be similar to python variant and respect method overriding)
- Add comment to "raise" statement why it looks so. Sure, someone will spend hours to understand what that mean.
There was a problem hiding this comment.
@kozzztik Thanks for the explanation!
When you try to inherit from Task on Py>3.6 (on 3.5 it is ok) that is not obvious, but you inherit CTask which not uses this method.
@asvetlov As I expected. It's not possible to use either method since 3.6.
Just remove it (may be also make C implementation be similar to python variant and respect method overriding)
Can't do that, Task inherits from Future. It's possible to remove them from C Task, but not from Python version.
Add comment to "raise" statement why it looks so. Sure, someone will spend hours to understand what that mean.
OK.
I'll change NotImplementedError to some other exception.
There was a problem hiding this comment.
Task does not support ... sounds slightly better to me but I don't insist.
|
ping |
f645e9d to
e11cf73
Compare
https://bugs.python.org/issue32363