Conversation
src/chat.rs
Outdated
| Ok(src_blob) => { | ||
| let src_path = src_blob.to_abs_path(); | ||
| let filename = param.get(Param::Filename).unwrap_or("file"); | ||
| match BlobObject::create_and_deduplicate( |
There was a problem hiding this comment.
FTR: This calls std::fs::copy() which supports FS-level deduplication, e.g. on Linux it does copy_file_range(). So on modern FSes like btrfs there's no big overhead
- rename chat var to be consistent with other tests
nicodh
force-pushed
the
ndh/forward-message-with-file
branch
from
9496491 to
42acf84
Compare
src/chat.rs
Outdated
| // because each account has its own blob directory. | ||
| if let Some(file_name) = param.get(Param::File) { | ||
| let src_blob = BlobObject::from_name(ctx_src, file_name) | ||
| .with_context(|| format!("Failed to get source blob: {file_name}"))?; |
There was a problem hiding this comment.
| .with_context(|| format!("Failed to get source blob: {file_name}"))?; | |
| .with_context(|| format!("Get source blob: {file_name}"))?; |
Usually "Failed to..." or "Error while..." already exists inside the error string so i'd avoid adding it, otherwise the final error looks like "Failed to get source blob: Failed to get blob path: ..."
src/chat.rs
Outdated
| let filename = param.get(Param::Filename).unwrap_or("file"); | ||
| let new_blob = | ||
| BlobObject::create_and_deduplicate(ctx_dst, &src_path, Path::new(filename)) | ||
| .context("Failed to copy blob file to destination account")?; |
There was a problem hiding this comment.
| .context("Failed to copy blob file to destination account")?; | |
| .context("BlobObject::create_and_deduplicate")?; |
Maybe even this way. Users won't understand the error message anyway, it's too technical and untranslated. I think it's important that some error is shown, but its content is for developers mainly.
There was a problem hiding this comment.
Personally, I do like understanding what went wrong just from reading the error message. Also, it's easier to grep for "Failed to copy blob file to destination account" than for "BlobObject::create_and_deduplicate".
| alice.send_msg(alice_self_chat.id, &mut msg).await; | ||
| let alice_self_msg = alice.get_last_msg().await; | ||
|
|
||
| // Delete the blob file from Alice's blobdir to simulate a missing file |
There was a problem hiding this comment.
Good as is, but consider merging this part into the previous test to reduce amount of code. Test failure reports will be less informative, but not sure that it's important currently
src/chat.rs
Outdated
| let filename = param.get(Param::Filename).unwrap_or("file"); | ||
| let new_blob = | ||
| BlobObject::create_and_deduplicate(ctx_dst, &src_path, Path::new(filename)) | ||
| .context("Failed to copy blob file to destination account")?; |
There was a problem hiding this comment.
Personally, I do like understanding what went wrong just from reading the error message. Also, it's easier to grep for "Failed to copy blob file to destination account" than for "BlobObject::create_and_deduplicate".
|
@adbenitez wants a core release, let's get this fix in first |
3 participants
resolves #7724