feat: Make AsyncArrowWriter accepts AsyncFileWriter #5753
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Xuanwo
commented
May 11, 2024
Xuanwo
force-pushed
the
add-async-file-writer
branch
from
a680933
to
4d0fe42
Compare
tustvold
reviewed
May 11, 2024
tustvold
reviewed
May 11, 2024
| /// | ||
| /// The underlying writer CAN decide to buffer the data or write it immediately. | ||
| /// This design allows the writer implementer to control the buffering and I/O scheduling. | ||
| fn write(&mut self, bs: Bytes) -> BoxFuture<'_, Result<()>>; |
There was a problem hiding this comment.
We should document atomicity requirements for this method
There was a problem hiding this comment.
I find current ArrowWriter's design is also not safe to retry. I will update the behavior notes of write to make it more clear and just take the Vec<u8> here to make this PR more simple.
Xuanwo
force-pushed
the
add-async-file-writer
branch
from
4d0fe42
to
723515d
Compare
|
Hi @tustvold, PTAL. Thanks! |
Signed-off-by: Xuanwo <github@xuanwo.io>
Xuanwo
force-pushed
the
add-async-file-writer
branch
from
723515d
to
2076737
Compare
3 tasks
|
Thank you |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Closes #5738
Rationale for this change
This makes
AsyncArrowWriteracceptsAsyncFileWriterso users can save an extra copy if they have a writer that can acceptBytesdirectly.What changes are included in this PR?
A bit change from the proposal but mainly the same.
Are there any user-facing changes?
AsyncArrowWriternow acceptsAsyncFileWriterinstead ofAsyncWrite. ButAsyncFileWriterhas been implemented for allAsyncWrite, so existing code should be working as before. For example, all testing code is not touched 馃拰Maybe we can catch up the arrow 52's train? #5688