Fix async client timeout and cancellation issues#77
Fix async client timeout and cancellation issues#77anuraaga wants to merge 18 commits intoconnectrpc:mainfrom
Conversation
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
7eac0a8 to
49562fc
Compare
…nto conformance-client-parallel
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
|
Ok @connectrpc/python sorry for the churn. Initially all I wanted was to speed up client conformance tests, but that also ended up being a stress test of the client which we haven't really done before (it's only num cpus * 4, in the future we may need even more stressful tests) - this showed some possible bugs that could surface to users, albeit for corner cases. And while I was at it, we already had client cancellation tests marked as flaky, but I could finally deep dive into them and I think they're actually fixed, at least as best as we can do. The change is large and complicated so it will be good to get eyes on it - @spenczar especially, I wonder if you've ran into patterns for dealing with async and cancellations cleanly before, maybe there's something besides the producer/consumer approach I took. |
| *_skipped_tests_async, | ||
| "--known-flaky", | ||
| "Client Cancellation/**", | ||
| "**/cancel-after-responses", |
There was a problem hiding this comment.
This is still flaky, I suspect it is an issue in the test harness, not code. I will defer debugging it as already did a lot of debugging and want to get the fixes in
stefanvanburen
left a comment
There was a problem hiding this comment.
stamping so we can get this in for #79; looks reasonable to me but not as familiar with the asyncio.Queue approach so up to you if you want to wait for a more thorough review.
httpx does not seem to reliably cancel I/O
Is there an upstream issue we can track? Or ought we open one (maybe hard to pinpoint)?
|
Thanks @stefanvanburen - it puts into perspective that this is a large workaround for likely an upstream issue. I am going to go ahead and defer this to another release to confirm it really is worth doing. |
Uh oh!
There was an error while loading. Please reload this page.