Add Python Cancellation Example #19465
Conversation
gnossen
added
area/documentation
lang/Python
release notes: yes
There was a problem hiding this comment.
My major concern is about the complexity overhead of the design. It's a great real-world usage, but it might be overkill for a demonstration of API usage.
Also, since many painful workaround is introduced by #19464, do you think you have some extra time to inspect the root cause?
| completed. | ||
|
|
||
| In the example, we use this interface to cancel our in-progress RPC when the | ||
| user interrupts the process with ctrl-c. |
There was a problem hiding this comment.
The signal mechanism is not a necessary step for cancellation. I think a simple future.cancel() is capable of convey the idea of cancelling an unary request.
There was a problem hiding this comment.
Discussed offline. If you have a better way to motivate why you would cancel on the client side, I'm more than happy to give it a shot.
| ``` | ||
|
|
||
| It's also important that you not block indefinitely on the RPC. Otherwise, the | ||
| signal handler will never have a chance to run. |
There was a problem hiding this comment.
This part is specific to our own implementation... Ideally, we should get rid of this bug?
There was a problem hiding this comment.
Yes. And I have added a TODO in the example to get rid of the workaround once the bug has been fixed. But until it has, this is the only way that this can be accomplished using our library.
| propagate a timeout to Python generators. As a result, simply iterating over the | ||
| results of the RPC can block indefinitely and the signal handler may never run. | ||
| Instead, we iterate over the generator on another thread and retrieve the | ||
| results on the main thread with a synchronized `Queue`. |
There was a problem hiding this comment.
Same as above, this is a workaround for our implementation defect. It's not that ideal to recommend it to our users...
There was a problem hiding this comment.
Agree it would be better to fix the bug. But until then, a workaround is the next best thing. This is an example. It can always be changed once our library supports a simpler workflow.
|
@lidizheng I've made changes to address some of your concerns. I'm looking into fixing the bug that necessitated the workarounds, but in the meantime could you PTALA at the other bits that I've changed since your last review? |
|
@lidizheng PTALA |
Related: #19464