Fallible stream for arrow-flight do_exchange call (#3462) #5698
Conversation
github-actions
bot
added
arrow
opensourcegeek
force-pushed
the
feat/3462
branch
from
283a7f1 to
6509de0
Compare
opensourcegeek
force-pushed
the
feat/3462
branch
2 times, most recently
from
f9ec823 to
8fe8a63
Compare
Signed-off-by: Praveen Kumar <[email protected]>
opensourcegeek
force-pushed
the
feat/3462
branch
from
8fe8a63 to
5cf10c2
Compare
There was a problem hiding this comment.
Looks good to me -- thank you @opensourcegeek 🙏
Since this is an API change I think it could be merged when we are ready for the next release with breaking API changes
| &mut self, | ||
| request: S, | ||
| ) -> Result<FlightRecordBatchStream> { | ||
| let request = self.make_request(request); | ||
| let (sender, mut receiver) = futures::channel::oneshot::channel(); |
There was a problem hiding this comment.
I wonder if there is some way to avoid the repetition here with the code in do_get above 🤔
There was a problem hiding this comment.
I did wonder about duplication of the code as in do_put method, making all errors (both in request stream and response stream) go back to client. Since it's my first MR here I didn't want to take it further. But happy to explore.
There was a problem hiding this comment.
Maybe we could do it as a follow on PR 🤔
I think this PR is ready to merge as long as we can accept API changes in this release (we are working to slow down the rate of breaking API changes - see #5368 )
Which issue does this PR close?
Closes #3462
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?