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

Added set_payload_override to UploadPartInputBuilder, PutObject, and GetObjectInputBuilder #1087

Open
2 tasks
jiashiwen opened this issue Mar 4, 2024 · 2 comments
Assignees
Labels
feature-request A feature should be added or improved. p2 This is a standard priority issue

Comments

@jiashiwen
Copy link

Describe the feature

Added set_payload_override to UploadPartInputBuilder, PutObject, and GetObjectInputBuilder to prevent frequent calls to sigv4 when using aws_sigv4::http_request::SignableBody to change object transfers, causing high CPU usage.

Use Case

  let payload_override = aws_sigv4::http_request::SignableBody::UnsignedPayload;
  let upload_part_res = client
        .upload_part()
        .set_payload_override(payload_override)
        .bucket(bucket)
        .key(key)
        .upload_id(upload_id)
        .body(stream)
        .part_number(p.part_num)
        .send()
        .await?;

Proposed Solution

impl UploadPartInputBuilder {
pub fn set_payload_override(mut self, payload_override: ::aws_sigv4::http_request::SignableBody) -> Self {
self.inner.signing_options.payload_override = payload_override;;
self
}
}

Other Information

#1085

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

A note for the community

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue, please leave a comment
@jiashiwen jiashiwen added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Mar 4, 2024
@rcoh
Copy link
Contributor

rcoh commented Mar 12, 2024

agreed we definitely need a way to support this. I think we'll need an RFC / design doc to investigate the best way to support this generally. I think this is probably possible with an interceptor today but definitely not the best user experience.

@rcoh rcoh added the p2 This is a standard priority issue label Mar 12, 2024
@jiashiwen
Copy link
Author

I have implemented this by modifying sdk/s3/src/operation/upload_part/builders.rs to add a new function send_with_plugins.
The modification code is as follows:

impl UploadPartFluentBuilder {
    pub async fn send(
        self,
    ) -> ::std::result::Result<
        crate::operation::upload_part::UploadPartOutput,
        ::aws_smithy_runtime_api::client::result::SdkError<
            crate::operation::upload_part::UploadPartError,
            ::aws_smithy_runtime_api::client::orchestrator::HttpResponse,
        >,
    > {
        let input = self
            .inner
            .build()
            .map_err(::aws_smithy_runtime_api::client::result::SdkError::construction_failure)?;
        let runtime_plugins = crate::operation::upload_part::UploadPart::operation_runtime_plugins(
            // self.handle.runtime_plugins.clone(),
            self.handle.runtime_plugins.clone().with_client_plugin(
                crate::presigning_interceptors::SigV4PresigningRuntimePlugin::new(
                    crate::presigning::PresigningConfig::expires_in(
                        std::time::Duration::from_secs(3000),
                    )
                    .unwrap(),
                    ::aws_sigv4::http_request::SignableBody::UnsignedPayload,
                ),
            ),
            &self.handle.conf,
            self.config_override,
        );
        crate::operation::upload_part::UploadPart::orchestrate(&runtime_plugins, input).await
    }

    pub async fn send_with_plugins(
        self,
        presigning_config: PresigningConfig,
    ) -> ::std::result::Result<
        crate::operation::upload_part::UploadPartOutput,
        result::SdkError<
            crate::operation::upload_part::UploadPartError,
            ::aws_smithy_runtime_api::client::orchestrator::HttpResponse,
        >,
    > {
        let input = self
            .inner
            .build()
            .map_err(result::SdkError::construction_failure)?;

        let plugin = crate::presigning_interceptors::SigV4PresigningRuntimePlugin::new(
            presigning_config,
            ::aws_sigv4::http_request::SignableBody::UnsignedPayload,
        );
        let runtime_plugins = crate::operation::upload_part::UploadPart::operation_runtime_plugins(
            // self.handle.runtime_plugins.clone(),
            self.handle
                .runtime_plugins
                .clone()
                .with_client_plugin(plugin),
            &self.handle.conf,
            self.config_override,
        );
        crate::operation::upload_part::UploadPart::orchestrate(&runtime_plugins, input).await
    }

}

Use case

let presigning = PresigningConfig::expires_in(std::time::Duration::from_secs(3000))?;
        let upload_part_res = self
            .client
            .upload_part()
            .upload_id(upload_id)
            .part_number(part_number)
            .bucket(bucket)
            .key(key)
            .body(stream)
            .send_with_plugins(presigning)
            .await?;

Is this a good user experience?
I can submit pr if officially allowed

@jmklix jmklix removed the needs-triage This issue or PR still needs to be triaged. label Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. p2 This is a standard priority issue
Projects
None yet
Development

No branches or pull requests

4 participants