GH-43837: [Go][IPC] Consolidate StreamWriter and FileWriter, ensuring that EOS indicator is written in file#43890
Conversation
|
|
|
All "Go producing, ..." scenarios now passing in integration testing workflow |
|
The change seems reasonable to me, but we need to figure out what's wrong with the integration tests here before we can merge this |
The integration tests were failing because I included commits from #43834 which breaks the tests for a number of implementations. I confirmed that the Go scenarios in the workflow were passing on this PR, even though other languages are still failing with the changes to the tests. I removed the commits now and the integration testing workflow succeeded. |
|
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 63b34c9. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 4 possible false positives for unstable benchmarks that are known to sometimes produce them. |
…suring that EOS indicator is written in file (apache#43890) ### Rationale for this change Fixes: apache#43837 Much of the logic between the ipc stream writer and the file writer was split. This PR changes the file writer so that it uses a stream writer internally, ensuring that a valid stream is embedded within the file. **TODO** - [x] Remove @ bkietz's commits ### What changes are included in this PR? - Refactor `fileWriter` to embed `streamWriter` and defer relevant methods - Add test ### Are these changes tested? Yes ### Are there any user-facing changes? Go-generated IPC files will contain the EOS indicator * GitHub Issue: apache#43837 Authored-by: Joel Lubinitsky <joellubi@gmail.com> Signed-off-by: Joel Lubinitsky <joellubi@gmail.com>
Rationale for this change
Fixes: #43837
Much of the logic between the ipc stream writer and the file writer was split. This PR changes the file writer so that it uses a stream writer internally, ensuring that a valid stream is embedded within the file.
TODO
What changes are included in this PR?
fileWriterto embedstreamWriterand defer relevant methodsAre these changes tested?
Yes
Are there any user-facing changes?
Go-generated IPC files will contain the EOS indicator