From efb88f94e31198c7359d37097b2e90e6c5772188 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 18:36:35 -0500 Subject: [PATCH] h5rs: check URL opens the remote file once open_arg_whole opened and parsed the remote file through open_arg, then opened it again to download it, so every `h5rs check URL` probed the server twice. It now opens the storage once and downloads through the same block cache (whose first block the probe already filled). Test: check --data of a file within one block costs exactly one request (two before). Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-tools/src/h5.rs | 11 +++++++---- crates/clawhdf5-tools/tests/remote.rs | 11 +++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/crates/clawhdf5-tools/src/h5.rs b/crates/clawhdf5-tools/src/h5.rs index c130f1f..7cba7c3 100644 --- a/crates/clawhdf5-tools/src/h5.rs +++ b/crates/clawhdf5-tools/src/h5.rs @@ -253,13 +253,14 @@ impl H5 { /// than `max_download` bytes is refused before anything is read: its /// length is only what the server claims. pub fn open_arg_whole(arg: &str, max_download: u64) -> Result
{ - let h5 = H5::open_arg(arg)?; - if h5.file.contiguous_bytes().is_some() { - return Ok(h5); + if !is_url(arg) { + return H5::open_arg(arg); } let name = shown(arg); #[cfg(feature = "remote")] { + // One open (one probe of the server); the download then reads + // through the same cache, the first block already in it. let storage = clawhdf5_remote::storage_for_url(arg, &clawhdf5_remote::Options::default()) .map_err(|e| Error::new(format!("{name}: {e}")))?; @@ -274,7 +275,9 @@ impl H5 { #[cfg(not(feature = "remote"))] { let _ = max_download; - unreachable!("open_arg refuses URLs without the remote feature") + Err(Error::new(format!( + "{name}: URLs need h5rs built with the `remote` feature" + ))) } } diff --git a/crates/clawhdf5-tools/tests/remote.rs b/crates/clawhdf5-tools/tests/remote.rs index 0ae0b40..de39814 100644 --- a/crates/clawhdf5-tools/tests/remote.rs +++ b/crates/clawhdf5-tools/tests/remote.rs @@ -158,3 +158,14 @@ fn credentials_in_urls_are_not_printed() { "{out}" ); } + +/// `check URL` opens the file once: for a file within the first block, +/// one request in all (it probed the server twice before). +#[test] +fn check_url_probes_the_server_once() { + let tall = Path::new(env!("CARGO_MANIFEST_DIR")).join("../clawhdf5/tests/fixtures/tall.h5"); + let server = server::Server::start(vec![("/t.h5".into(), std::fs::read(&tall).unwrap())]); + let (out, rc) = h5rs(&["check", "--data", &server.url("/t.h5")]); + assert_eq!(rc, 0, "{out}"); + assert_eq!(server.requests(), 1, "{:?}", server.log()); +}