feat(scorpio): complete MR layer creation#1233
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Pull Request Overview
This PR completes the MR layer creation by fetching MR file lists, processing file actions (including whiteout for deletions), and downloading necessary files, then integrates this into the daemon's mount handler.
- build_mr_layer now sets up directories, handles “new”, “modified”, and “deleted” actions, and queues files for download.
- Added
download_mr_filesin the fetch module for batch MR file downloads. - Updated mount_handler to invoke
build_mr_layer, handle errors, and pass MR flag to overlay_mount.
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| scorpio/src/manager/mr.rs | Implement MR layer creation logic and action processing. |
| scorpio/src/manager/fetch.rs | Add download_mr_files for concurrent MR file downloads. |
| scorpio/src/manager/mod.rs | Fix duplicate pub mod mr; declarations. |
| scorpio/src/daemon/mod.rs | Invoke MR layer build in mount_handler and propagate errors. |
| scorpio/src/fuse/mod.rs | Reformat overlay_mount calls for readability. |
| mercury/src/internal/object/commit.rs | Remove extraneous blank lines. |
| mercury/src/internal/object/blob.rs | Remove unnecessary blank line. |
| mercury/src/internal/model/commit.rs | Simplify import block formatting. |
| libra/tests/command/log_test.rs | Add missing comma in test struct literal. |
| libra/src/command/commit.rs | Remove trailing blank line. |
| jupiter/src/service/mod.rs | Remove trailing whitespace. |
Comments suppressed due to low confidence (3)
scorpio/src/manager/fetch.rs:332
- [nitpick] Consider adding unit or integration tests for download_mr_files to verify that files are correctly enqueued and downloaded under MR scenarios, ensuring this new functionality is covered.
pub async fn download_mr_files(
scorpio/src/manager/mr.rs:43
- strip_prefix expects a string slice (&str) pattern; passing a character ('/') may not compile. Use strip_prefix("/") to correctly remove a leading slash.
let relative_path = file.path.strip_prefix('/').unwrap_or(&file.path);
scorpio/src/manager/mr.rs:55
- Ensure the action string matches the API specification (original code used "remove"); if the API returns "remove" instead of "deleted", this branch will not be executed and fall into the unknown action case.
"deleted" => {
| config::base_url(), | ||
| link | ||
| ); | ||
| async fn fetch_files_list(link: &str) -> Result<FilesListResp, reqwest::Error> { |
There was a problem hiding this comment.
The implementation of fetch_files_list no longer calls .send().await, .error_for_status(), or .json(), so it returns a request builder instead of sending the HTTP request and parsing JSON. Restore the request chain to perform the request and deserialize the response.
|
|
||
| if result != 0 { | ||
| let err = std::io::Error::last_os_error(); | ||
| eprintln!("Failed to create whiteout file {}: {}", file.path, err); |
There was a problem hiding this comment.
[nitpick] Currently the error when creating a whiteout file is only logged; consider propagating this error via the function's Result to ensure upstream error handling instead of silently continuing.
| eprintln!("Failed to create whiteout file {}: {}", file.path, err); | |
| return Err(Box::new(std::io::Error::new( | |
| err.kind(), | |
| format!("Failed to create whiteout file {}: {}", file.path, err), | |
| ))); |
|
|
||
| // Create whiteout as character device with 0/0 device number | ||
| // This is the standard way overlay filesystems recognize deleted files | ||
| let mode = libc::S_IFCHR | 0o777; |
There was a problem hiding this comment.
[nitpick] The mode 0o777 is a magic number; consider defining a named constant or adding a comment explaining why this permission is chosen to improve readability.
| let mode = libc::S_IFCHR | 0o777; | |
| const WHITEOUT_FILE_PERMISSIONS: u32 = 0o777; // Full read, write, and execute permissions for whiteout files | |
| let mode = libc::S_IFCHR | WHITEOUT_FILE_PERMISSIONS; |
Resolves #1224
Completed the setup for MR layer support, which includes fetching the files for a single MR and performing differentiated processing.