Skip to content

ARROW-5034: [C#] ArrowStreamWriter and ArrowFileWriter implement sync WriteRecordBatch#8146

Closed
suhsteve wants to merge 2 commits intoapache:masterfrom
suhsteve:writerecordbatch
Closed

ARROW-5034: [C#] ArrowStreamWriter and ArrowFileWriter implement sync WriteRecordBatch#8146
suhsteve wants to merge 2 commits intoapache:masterfrom
suhsteve:writerecordbatch

Conversation

@suhsteve
Copy link
Copy Markdown
Contributor

@suhsteve suhsteve commented Sep 9, 2020

ArrowStreamWriter and ArrowFileWriter currently only supports an WriteRecordBatchAsync. This PR implements a synced version of it. All the logic has been reused and all async calls have been replaced with sync counterparts.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 9, 2020

@eerhardt eerhardt self-requested a review September 9, 2020 14:36
@suhsteve suhsteve changed the title ARROW-9951: [C#] ArrowStreamWriter implement sync WriteRecordBatch ARROW-5034: [C#] ArrowStreamWriter implement sync WriteRecordBatch Sep 9, 2020
@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 9, 2020

Copy link
Copy Markdown
Contributor

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great @suhsteve! Thanks for helping with this.

I have a couple comments, once they are addressed I believe we should be able to merge this.

Copy link
Copy Markdown
Contributor

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. Thanks for the contribution, @suhsteve.

I'll merge this tomorrow if I don't see anymore feedback in the meantime.

@suhsteve suhsteve changed the title ARROW-5034: [C#] ArrowStreamWriter implement sync WriteRecordBatch ARROW-5034: [C#] ArrowStreamWriter and ArrowFileWriter implement sync WriteRecordBatch Sep 11, 2020
@eerhardt eerhardt closed this in 974a74d Sep 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants