Conversation
Signed-off-by: Jose Nino <jnino@lyft.com>
|
@buildbreaker @rebello95 I updated with @goaway's suggestion. lmk what you think |
| public func close(_ data: Data) { | ||
| // The gRPC protocol requires the client stream to close with a DATA frame. | ||
| // More information here: https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#requests | ||
| self.underlyingEmitter.close() |
There was a problem hiding this comment.
This is actually wrong, it'll still send trailers here rather than empty data.
There was a problem hiding this comment.
How come? I thought by deleting the extension below, I was getting rid of a default value when no argument was given
There was a problem hiding this comment.
This comment was before the extension was deleted. I don't think this compiles as-is, you'll need to specify trailers: nil I believe
There was a problem hiding this comment.
😂 I think my commit and your comment cross paths. Will update
|
Let's make the appropriate changes on Android as well as part of this PR |
| public func close() { | ||
| self.close(trailers: [:]) | ||
| } | ||
| /// - parameter trailers: optional trailers to send over the stream. |
There was a problem hiding this comment.
Suggestion: Trailers with which to close the stream. If nil, stream will be closed with an empty data frame.
| } | ||
|
|
||
| func close(trailers: [String: [String]]) {} | ||
| func close(trailers: [String: [String]]?) {} |
There was a problem hiding this comment.
I’ll add a test for this in another PR
Description: the gRPC protocol requires closing the client stream in a DATA frame. This PR fixes the GRPCStreamEmitter for iOS.
Risk Level: med - protocol level changes.
Testing: local.
Signed-off-by: Jose Nino jnino@lyft.com