GH-39574: [Go] Enable PollFlightInfo in Flight RPC#39575
GH-39574: [Go] Enable PollFlightInfo in Flight RPC#39575lidavidm merged 2 commits intoapache:mainfrom
Conversation
b8dc3e4 to
47cdc36
Compare
|
CC @zeroshade |
| if _, err = pstream.Recv(); err != nil && err != io.EOF { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
didn't we have a helper function that did this?
There was a problem hiding this comment.
I don't think so. This is basically verbatim from the synchronous version of this method.
go/arrow/flight/flightsql/client.go
Outdated
| return p.client.Client.PollFlightInfo(ctx, desc, opts...) | ||
| } | ||
| return p.client.Client.PollFlightInfo(ctx, retryDescriptor, opts...) |
There was a problem hiding this comment.
couldn't we condense this by doing retryDescriptor = desc and not duplicate the line?
There was a problem hiding this comment.
Either that, or can we simplify and early return if retryDescriptor != nil { return p.client.Client.PollFlightInfo.... }
zeroshade
left a comment
There was a problem hiding this comment.
Just a few nitpicks but otherwise looks good to me
|
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 143a7da. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 1 possible false positive for unstable benchmarks that are known to sometimes produce them. |
### Rationale for this change It's impossible to use the current bindings with PollFlightInfo. Required for apache/arrow-adbc#1457. ### What changes are included in this PR? Add new methods that expose PollFlightInfo. ### Are these changes tested? Yes ### Are there any user-facing changes? Adds new methods. * Closes: apache#39574 Authored-by: David Li <li.davidm96@gmail.com> Signed-off-by: David Li <li.davidm96@gmail.com>
|
@github-actions crossbow submit verify-rc-source-integration-linux-* |
|
I am verifying if this caused: #41201 |
|
Revision: 2d89a1c Submitted crossbow builds: ursacomputing/crossbow @ actions-c07bcbee39
|
Requires apache/arrow#39575. Closes #1451.
Rationale for this change
It's impossible to use the current bindings with PollFlightInfo. Required for apache/arrow-adbc#1457.
What changes are included in this PR?
Add new methods that expose PollFlightInfo.
Are these changes tested?
Yes
Are there any user-facing changes?
Adds new methods.