-
Notifications
You must be signed in to change notification settings - Fork 111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add into_inner() on Rust writer #1314
Conversation
rust/src/write.rs
Outdated
.finalize() | ||
.0 | ||
.finish() | ||
.expect("compression error") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean to have this here instead of returning a result?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. It's because of the zstd compressor. I'll add a comment to clarify.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if we swallowed the error instead? The caller is already expecting to not get a complete MCAP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've rejigged it to avoid the unwrap entirely by using the interface in zio::Writer instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@james-rms swallowing wasn't an option because the wrapper interface wouldn't even let you get the W back unless finishing succeeded.
6a19bdb
to
d2cfc83
Compare
Changelog
Add an
into_inner()
method on the Rust writer to get back the underlying stream.Docs
https://linear.app/foxglove/issue/FG-9969/[rust-sdk]-cant-close-mcap-writer
Description
Some I/O errors only show up when the output file is closed. This change allows users to check for such errors.
Fixes: FG-9969