feat: make start_clean size parameter optional for streaming uploads#754
feat: make start_clean size parameter optional for streaming uploads#754
Conversation
Change the `size` parameter of `FileUploadSession::start_clean()` from `u64` to `Option<u64>`. Passing `None` signals that the final file size is unknown (FUSE streaming uploads), which prevents `debug_assert` panics when `completed_bytes` exceeds the initially declared `total_bytes=0`. This is a breaking change to the `start_clean` signature. All existing callers are updated to wrap the size argument in `Some(...)`.
- Replace redundant closure with direct function reference (python.rs) - Use std::f32::consts::PI instead of hardcoded 3.14f32 (configuration_utils.rs) - Remove useless assert!(true) in test (sync_primatives.rs) - Replace assert!(false) with panic!() (par_utils.rs)
rajatarya
left a comment
There was a problem hiding this comment.
Solid change. Option<u64> is the right type for "size may not be known" — fixes the debug assertion panic on FUSE streaming uploads cleanly.
All callers updated consistently, docs updated, and the clippy bonus fixes (assert!(false, ...) → panic!(), removing assert!(true), closure → function ref, 3.14f32 → PI) are all welcome.
One note: upload_files (the batch API at file_upload_session.rs:114) still calls register_new_file(updater, Some(file_size)) directly without going through start_clean. Not a bug since it reads actual size from fs::metadata, but worth keeping in mind if this path ever gets refactored — there are now two separate registration paths.
Since this is a breaking change to upload_file() on UploadCommit, worth coordinating merge order with #730 (hf-mount will be a consumer of this API).
Summary
Make the
sizeparameter ofFileUploadSession::start_clean()optional by changing fromu64toOption<u64>. This allows FUSE streaming uploads where the final file size is unknown, preventing debug assertion panics whencompleted_bytesexceeds the initially declaredtotal_bytes=0.Changes
start_clean()signature now takesOption<u64>for sizeSome(...)upload_file()to useOption<u64>Some()wrapperBreaking Changes
FileUploadSession::start_clean()signature changed fromsize: u64tosize: Option<u64>Note
Medium Risk
This is a breaking API change across upload/cleaning entry points (
start_clean/upload_file) that affects progress tracking and completion accounting; incorrectNonehandling could cause misreported progress or assertions at finalize. Most call sites/tests are updated mechanically, lowering risk to runtime behavior beyond the new unknown-size path.Overview
Makes streaming upload size optional by changing
FileUploadSession::start_clean(andUploadCommit::upload_file/upload_file_blocking) to takeOption<u64>so callers can stream when the final size is unknown.Updates all internal call sites, examples, and integration/unit tests to pass
Some(size)where sizes are known, and wires the optional size through to the completion/progress tracker registration.Includes a few small cleanup tweaks unrelated to the API change (minor error mapping simplification in python config bindings, test assertion style improvements, and replacing a float literal with
std::f32::consts::PI).Written by Cursor Bugbot for commit 16447da. This will update automatically on new commits. Configure here.