Update AWS SDK to 1.x #684
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Reviewable status: 0 of 2 LGTMs obtained, and pending CI: Analyze (javascript-typescript), Analyze (python), Bazel Dev / ubuntu-22.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Local / ubuntu-22.04, Remote / large-ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), macos-13, pre-commit-checks, publish-image, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, vale, windows-2022 / stable, zig-cc ubuntu-20.04, zig-cc ubuntu-22.04 (waiting on @adam-singer and @allada)
There was a problem hiding this comment.
Reviewed 11 of 11 files at r1, all commit messages.
Reviewable status: 1 of 2 LGTMs obtained, and pending CI: Bazel Dev / ubuntu-22.04, Cargo Dev / ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), zig-cc ubuntu-20.04, zig-cc ubuntu-22.04 (waiting on @adam-singer)
nativelink-store/src/s3_store.rs line 205 at r1 (raw file):
match result { Ok(head_object_output) => match head_object_output.content_length { Some(nonneg_val) if nonneg_val >= 0 => {
nit: This is really hard to read. Can we do this as:
Ok(head_object_output) => {
let Some(val) = head_object_output.content_length else {
return Some((RetryResult::Ok(None), state)); // Retry.
}
if val >= 0 {
return Some((RetryResult::Ok(Some(nonneg_val as usize)), state));
}
return Some((
RetryResult::Err(make_err!(
Code::InvalidArgument,
"Negative content length in S3: {nonneg_val:?}",
)),
state,
));
},
There was a problem hiding this comment.
Reviewed 4 of 6 files at r2.
Reviewable status: 1 of 2 LGTMs obtained, and pending CI: Bazel Dev / ubuntu-22.04 (waiting on @adam-singer)
nativelink-store/src/s3_store.rs line 206 at r2 (raw file):
Ok(head_object_output) => { let Some(length) = head_object_output.content_length else { return Some((RetryResult::Ok(None), state)); // Retry.
nit: Opps this is not a Retry.
The main change here is that we no longer differentiate between "other" and `Unhandled` errors. The distinction didn't make sense in the first place and is now handled uniformly as a retry case.
The main change here is that we no longer differentiate between "other" and
Unhandlederrors. The distinction didn't make sense in the first place and is now handled uniformly as a retry case.Towards #460
This change is