convert cpapi_artifact_download to use dropshot / progenitor streaming interfaces
#1442
Conversation
sled-agent/src/updates.rs
Outdated
| let contents = response | ||
| .into_inner_stream() | ||
| .try_fold(Vec::new(), |mut acc, x| async move { | ||
| acc.extend(x); | ||
| Ok(acc) | ||
| }) | ||
| .await | ||
| .map_err(|e| Error::Response(e.into()))?; |
There was a problem hiding this comment.
This should be easier to stream now if we want, e.g.
let mut file = tokio::fs::File::open(&tmp_path).await?;
let mut stream = response.into_inner_stream();
while let Some(chunk) = stream.next().await {
file.write_all(&chunk?);
}(adding .map_err as appropriate). Alternately you could do something similar with .try_for_each on the stream.
There was a problem hiding this comment.
so I played around with that a bit, but didn't land on something I loved. I'll try again as you suggest.
What should we be doing if we encounter an error part way through writing the file? I don't see us removing that temp file.
There was a problem hiding this comment.
Ok, I couldn't make try_for_each work... I don't think I understand how to use file from within the closure properly. But I used a while loop as you suggested.
There was a problem hiding this comment.
What should we be doing if we encounter an error part way through writing the file? I don't see us removing that temp file.
Hm. I suppose we could use tempfile here... I have some other code for a side project that is using that combined with tokio::fs::File. I'll push something on top of this branch unless you want to give it a shot?
I used NamedTempFile::into_parts and then tokio::fs::File::from_std. Once the TempPath from into_parts gets dropped the file is cleaned up. When everything goes well without errors, TempPath::persist can be used to move it into the proper place.
There was a problem hiding this comment.
I'll leave it to you; can you CC me on the review? I'd love to see.
Crucible changes:
Make crutest use BlockIO trait instead of a Guest (#1452)
Use new syncfs syscall (#1427)
Increment write backpressure before deferred encryption (#1444)
Use RAII handles for backpressure (#1443)
Fine-tuning backpressure clamping, and API cleanups (#1442)
Fix outdated comment (#1447)
Update Rust crate rusqlite to 0.32 (#1418)
Fix write reordering bug (#1448)
Remove `history_file` (#1446)
Remove optionality for `BlockRes` in `DeferredWrite` (#1441)
Propolis changes
instance spec rework: tighten up component naming (#761)
instance spec rework: remove most dependencies on `InstanceSpecV0` from propolis-server (#757)
fix new 1.81.0 warning and clippy error (#760)
standalone: be more helpful with bad block device configs (#758)
Crucible changes:
Make crutest use BlockIO trait instead of a Guest (#1452)
Use new syncfs syscall (#1427)
Increment write backpressure before deferred encryption (#1444)
Use RAII handles for backpressure (#1443)
Fine-tuning backpressure clamping, and API cleanups (#1442)
Fix outdated comment (#1447)
Update Rust crate rusqlite to 0.32 (#1418)
Fix write reordering bug (#1448)
Remove `history_file` (#1446)
Remove optionality for `BlockRes` in `DeferredWrite` (#1441)
Propolis changes
instance spec rework: tighten up component naming (#761)
instance spec rework: remove most dependencies on `InstanceSpecV0` from
propolis-server (#757)
fix new 1.81.0 warning and clippy error (#760)
standalone: be more helpful with bad block device configs (#758)
Co-authored-by: Alan Hanson <alan@oxide.computer>
This lets us turn on
test_write_artifact_to_filesystemwhich was previously ignored. I hope this is a good thing.