fix(client): Use Stream instead of TrySteam for client calls#61
fix(client): Use Stream instead of TrySteam for client calls#61LucioFranco merged 9 commits intohyperium:masterfrom alce:input-streams
Stream instead of TrySteam for client calls#61Conversation
|
Updating interop was actually very easy. This is ready for review. |
Can you expand on that? Why isn't it necessary? How does a client get notified of a stream failure (with its status/error message)? |
|
Hi @olix0r, maybe the title is misleading. All this PR does is remove the need to wrap outgoing messages in So instead of this: client.method(stream::iter(vec![Ok(1), Ok(2), Ok(3)])).await?;we can write this client.method(stream::iter(vec![1, 2, 3])).await?It is totally possible that I'm missing something but I don't understand the semantics of potentially sending an If the server tries to read all messages in the stream before returning a response, we don't even know how many messages, (or which ones) it managed to process. |
|
Ah! I see: this is for request streams and not response streams. That makes sense, then. |
|
I think we might want to update this function to not take a result? https://github.com/hyperium/tonic/blob/master/tonic/src/codec/encode.rs#L32 |
|
@alce I must be tired I totally missed that! 😅 |
Stream instead of TrySteam for client calls
Motivation
Currently, client methods take
TryStreamsas inputs when it's not really necessary. This change makes client calls a bit simpler and potentially helps simplify a solution for #34.Solution
This PR is a POC that changes input stream bounds from
Stream<Item = Result<Message, Status>>toStream<Item = Message>. As is, client and servers work fine but interop tests need to be adjusted for this PR to be complete.