Conversation
Implements single-file Upload and Download for the SEP-compliant S3 Transfer Manager (aws-cpp-sdk-s3-transfer), backed by aws-c-s3 meta requests via the aws-crt-cpp S3 bindings. Covers file and stream sources/sinks, progress and finish callbacks, checksum config, cancellation, and CRT-to-SEP error mapping.
|
|
||
| // Zero-copy stream download. Each part is delivered to the receiver in object order. | ||
| explicit DownloadRequest( | ||
| Aws::S3::Model::GetObjectRequest s3Request, |
There was a problem hiding this comment.
Not a big fan of requiring to construct with a different request type, thats not a preferable UX imo
| // StandardHttpResponse's constructor eagerly invokes the request's response-stream factory to | ||
| // materialize its body stream; without this the factory is empty and aborts under -fno-exceptions. | ||
| httpRequest->SetResponseStreamFactory(Aws::Utils::Stream::DefaultResponseStreamFactoryMethod); | ||
| std::shared_ptr<Aws::Http::HttpResponse> response = |
There was a problem hiding this comment.
Do we have to construct a temp request and a temp response? Can we not just construct an AwsError directly since that's all this function needs to return
There was a problem hiding this comment.
We could technically construct an AwsError, but we'd still need to derive the actual error type which happens at the return with Aws::Client::S3ErrorMarshaller().BuildAWSError(response);
BuildAwsError takes an HttpResponse as input so we build a temp one and populate it with the CRT's status/body to parse the XML body into the proper S3Error.
| std::promise<OutcomeT> promise; | ||
| RequestT request; | ||
| uint64_t totalBytes = 0; | ||
| bool totalBytesHasBeenSet = false; |
There was a problem hiding this comment.
does totalBytes not need to be atomic?
There was a problem hiding this comment.
totalBytes doesn't need to be atomic here because its only set once per dispatch for upload/download (download is set through the SetProgressCallback from crt). The user doesn't have a direct copy and outside of dispatch nothing ever updates or writes to it.
| std::atomic<int> completeCount{0}; | ||
| std::atomic<int> failedCount{0}; | ||
| std::atomic<uint64_t> lastTransferredBytes{0}; | ||
| std::atomic<uint64_t> maxTransferredBytes{0}; |
There was a problem hiding this comment.
nit: if the values cannot be negative and are purely for counting, use size_t
2 participants
Issue #, if available:
Description of changes:
Single File Upload/Download for TM 2.0
Check all that applies:
Check which platforms you have built SDK on to verify the correctness of this PR.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.