Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

s3 tm upload req resp #3761

Merged
merged 1 commit into from
Jul 16, 2024
Merged

s3 tm upload req resp #3761

merged 1 commit into from
Jul 16, 2024

Conversation

aajtodd
Copy link
Contributor

@aajtodd aajtodd commented Jul 15, 2024

Description

Follow up PR to #3756 that is one part of an initial multipart upload support.

This PR is simple, it just defines new request and response types for the upload API. These are nearly identical to what is found in the aws_sdk_s3 crate with the exception of the body type that leverages the new InputStream type (naming bikeshed still planned for later) for the body rather than ByteStream.


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@aajtodd aajtodd requested a review from a team as a code owner July 15, 2024 20:29
Copy link

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

@aajtodd aajtodd merged commit 18e5dfd into s3-tm-main Jul 16, 2024
41 of 45 checks passed
@aajtodd aajtodd deleted the atodd/s3-upload-req-resp branch July 16, 2024 14:23
*/

pub use crate::upload::request::UploadRequest;
pub use crate::upload::response::UploadResponse;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd hesitate to name these "Request" and "Response" when they don't correlate 1:1 with HTTP requests and responses. It makes the library confusing to talk about, internally and with customers. This happens all the time talking about "meta requests" in aws-c-s3.

Instead of Request, maybe: UploadOptions? UploadSettings? UploadConfig? UploadParams?
Instead of Response, maybe: UploadOutcome? UploadSummary? UploadReport?

/// </note> <note>
/// <p>This functionality is not supported for directory buckets.</p>
/// </note>
pub content_md5: Option<String>,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't allow this, or checksum_crc32 and friends, since it might get sent MPU

/// Common response fields for uploading an object to Amazon S3
#[non_exhaustive]
#[derive(Clone, PartialEq)]
pub struct UploadResponse {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll want to expose the Parts, too. So users can access the per-part checksums

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants