refactor(rust): Introduce Writeable and AsyncWriteable
#21599
Conversation
github-actions
bot
added
internal
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #21599 +/- ##
==========================================
- Coverage 80.24% 80.15% -0.10%
==========================================
Files 1604 1604
Lines 231329 231401 +72
Branches 2639 2639
==========================================
- Hits 185635 185483 -152
- Misses 45085 45309 +224
Partials 609 609 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
nameexhaustion
force-pushed
the
writeable
branch
from
16de202 to
3cb7e64
Compare
crates/polars-io/src/utils/file.rs
Outdated
| eprintln!("try_get_writeable: forced async converted path: {}", path) | ||
| pub fn close(self) -> PolarsResult<()> { | ||
| match self { | ||
| // @RAISE_FILE_CLOSE_ERR |
There was a problem hiding this comment.
Left this todo for now // @RAISE_FILE_CLOSE_ERR (here and 1 place down below)
It can be updated after #21588
nameexhaustion
requested review from
ritchie46,
orlp and
c-peters
as code owners
| /// | ||
| /// Also see: `Writeable::into_async_writeable` and `AsyncWriteable`. | ||
| pub enum Writeable { | ||
| Local(std::fs::File), |
There was a problem hiding this comment.
Could we theoretically (in a follow up PR) add an in-memory buffer to this? This would allow sinks to also output to in-memory buffers.
There was a problem hiding this comment.
Should be, we can do it later when we decide to implement sinking to memory
nameexhaustion
force-pushed
the
writeable
branch
from
3e23202 to
5dbaf9e
Compare
| Ok(()) => {}, | ||
| e @ Err(_) => { | ||
| if std::thread::panicking() { | ||
| eprintln!("ERROR: CloudWriter errored on close: {:?}", e) |
There was a problem hiding this comment.
drive-by, just print an error message if already panicking to avoid abort
| eprintln!("try_get_writeable: forced async converted path: {}", path) | ||
| pub fn close(self) -> std::io::Result<()> { | ||
| match self { | ||
| Self::Local(v) => ClosableFile::from(v).close(), |
There was a problem hiding this comment.
Local files closed here
| match self { | ||
| Self::Local(v) => async { | ||
| let f = v.into_std().await; | ||
| ClosableFile::from(f).close() |
There was a problem hiding this comment.
Local files closed here (AsyncWriteable)
Prepares for cloud support on new-streaming
Also implementsAsyncWriteonto theCloudWriter.A
Writeablecan generally be directly used for writing:It can be
Derefed if a trait object is needed:(similarly with
AsyncWriteable)