Gracefully exit with and on GoAway messages #54
Conversation
|
@mattbennett |
nameko_grpc/streams.py
Outdated
|
|
||
| When a stream is closed we append the STREAM_END item or a GrpcError, so an | ||
| exhausted stream will possibly still have 1 item left in the queue, so we | ||
| must check for that. |
There was a problem hiding this comment.
This doesn't seem to be reflected in the implementation?
There was a problem hiding this comment.
You're correct, the comment is not needed.
| @@ -166,7 +170,7 @@ def headers_to_send(self, defer_until_data=True): | |||
| if self.headers_sent or len(self.headers) == 0: | |||
| return False | |||
|
|
|||
| if defer_until_data and self.queue.empty(): | |||
| if defer_until_data and self.queue.empty() and self.buffer.empty(): | |||
There was a problem hiding this comment.
Was this a preexisting bug, or did the implementation need to change as a result of the termination handling?
There was a problem hiding this comment.
Removed this as It doesn't seem like it was needed and I don't see why I added it.
There was a problem hiding this comment.
actually I've added it back.
Previously, we wouldn't send headers until there was data to send (which sat on the queue as it wasn't flushed until the headers had been sent. But now, as a result of calling flush_queue_to_buffer indiscriminately, that data might be on the buffer rather than the queue.
We could possibly remove the check on the queue empty but it feels safer to keep it.
remove uneeded comment
|
Hi @mattbennett - we're planning on merging this as we think it's working well in production now. Shout if you have any issue 😄 |
Summary
Http2 supports the concept of a GOAWAY frame
https://www.rfc-editor.org/rfc/rfc7540#section-6.8
which our underlying library (h2) doesn't handle correctly (python-hyper/h2#1181)
This is patches in support to h2 and then we handle it correctly from our side.