Skip to content

feat: make start_clean size parameter optional for streaming uploads#754

Open
XciD wants to merge 4 commits intomainfrom
adrien/start-clean-optional-size
Open

feat: make start_clean size parameter optional for streaming uploads#754
XciD wants to merge 4 commits intomainfrom
adrien/start-clean-optional-size

Conversation

@XciD
Copy link
Copy Markdown
Member

@XciD XciD commented Mar 25, 2026

Summary

Make the size parameter of FileUploadSession::start_clean() optional by changing from u64 to Option<u64>. This allows FUSE streaming uploads where the final file size is unknown, preventing debug assertion panics when completed_bytes exceeds the initially declared total_bytes=0.

Changes

  • start_clean() signature now takes Option<u64> for size
  • All existing callers updated to wrap size argument in Some(...)
  • Updated public API upload_file() to use Option<u64>
  • Fixed test calls to use Some() wrapper

Breaking Changes

  • FileUploadSession::start_clean() signature changed from size: u64 to size: 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; incorrect None handling 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 (and UploadCommit::upload_file / upload_file_blocking) to take Option<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.

XciD added 4 commits March 20, 2026 22:58
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)
Copy link
Copy Markdown
Collaborator

@rajatarya rajatarya left a comment

Choose a reason for hiding this comment

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

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.14f32PI) 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).

@XciD XciD marked this pull request as ready for review April 1, 2026 15:29
@XciD XciD requested review from hoytak and seanses April 1, 2026 15:30
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