Skip to content

Commit

Permalink
Update openDAL to address AWS/GHAC issues (#21779)
Browse files Browse the repository at this point in the history
0.44.2 should fix the GHAC AWS S3 errors I'm seeing, but we might as
well update to the latest version that's easy. 0.46 brings some bigger
breaking changes, so that version is 0.45.1
  • Loading branch information
DLukeNelson authored Jan 7, 2025
1 parent 10a1699 commit fd3f4c9
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 80 deletions.
2 changes: 2 additions & 0 deletions docs/notes/2.25.x.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ one in `remote_store_headers` or `remote_execution_headers`.

Pants now supports the `{chroot}` replacement marker in remote execution contexts. (With local and Docker execution, the `{chroot}` marker is replaced with the absolute path of the sandbox directory if it appears in program arguments or environment variables. Pants will do the same as well in remote execution contexts. This requires `/bin/bash` to be available on the remote execution server.)

The OpenDAL library powering the Github Actions cache backend has been updated, picking up some bug fixes for Github Enterprise Server instances using AWS S3 as backing storage for the Github Actions cache.

### New Options System

The "legacy" options system is removed in this release. All options parsing is now handled by the new, native parser.
Expand Down
110 changes: 40 additions & 70 deletions src/rust/engine/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/rust/engine/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ notify = { git = "https://github.com/pantsbuild/notify", rev = "276af0f3c5f300bf
num_cpus = "1"
num_enum = "0.5"
once_cell = "1.20"
opendal = { version = "0.41", default-features = false, features = [
opendal = { version = "0.45.1", default-features = false, features = [
"services-memory",
"services-fs",
"services-ghac",
Expand Down
22 changes: 13 additions & 9 deletions src/rust/engine/remote_provider/remote_provider_opendal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,7 @@ impl Provider {
// TODO: record Metric::RemoteStoreRequestTimeouts for timeouts
TimeoutLayer::new()
.with_timeout(options.timeout)
// TimeoutLayer requires specifying a non-zero minimum transfer speed too.
.with_speed(1),
.with_io_timeout(options.timeout),
)
// TODO: RetryLayer doesn't seem to retry stores, but we should
.layer(RetryLayer::new().with_max_times(options.retries + 1))
Expand All @@ -78,7 +77,7 @@ impl Provider {

pub fn fs(path: &str, scope: String, options: RemoteStoreOptions) -> Result<Provider, String> {
let mut builder = opendal::services::Fs::default();
builder.root(path).enable_path_check();
builder.root(path);
Provider::new(builder, scope, options)
}

Expand Down Expand Up @@ -161,19 +160,24 @@ impl Provider {

match mode {
LoadMode::Validate => {
let correct_digest = async_verified_copy(digest, false, &mut reader, destination)
.await
.map_err(|e| format!("failed to read {}: {}", path, e))?;
let correct_digest =
match async_verified_copy(digest, false, &mut reader, destination).await {
Ok(correct_digest) => correct_digest,
Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(false),
Err(e) => return Err(format!("failed to read {}: {}", path, e)),
};

if !correct_digest {
// TODO: include the actual digest here
return Err(format!("Remote CAS gave wrong digest: expected {digest:?}"));
}
}
LoadMode::NoValidate => {
tokio::io::copy(&mut reader, destination)
.await
.map_err(|e| format!("failed to read {}: {}", path, e))?;
match tokio::io::copy(&mut reader, destination).await {
Ok(result) => result,
Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(false),
Err(e) => return Err(format!("failed to read {}: {}", path, e)),
};
}
}
Ok(true)
Expand Down

0 comments on commit fd3f4c9

Please sign in to comment.