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

Fix SDK::Endpoint built-in #2935

Merged
merged 2 commits into from
Aug 22, 2023

Conversation

jdisanti
Copy link
Collaborator

Setting endpoint_url on a SDK Config currently has no effect due to a bug in deciding if a parameter is a built-in or not in EndpointBuiltinsDecorator. The generated code is placing the endpoint URL into the ConfigBag with the correct aws_types::endpoint_config::EndpointUrl type, but then the operation runtime plugin is trying to pull it out of the ConfigBag with the crate::config::EndpointUrl type, which always yields nothing.

Or, to illustrate with the generated code itself:

    pub fn set_endpoint_url(&mut self, endpoint_url: Option<::std::string::String>) -> &mut Self {
        self.config.store_or_unset(endpoint_url.map(::aws_types::endpoint_config::EndpointUrl));
        self
    }

vs.

let params = crate::config::endpoint::Params::builder()
            .set_endpoint(cfg.load::<crate::config::EndpointUrl>().map(|ty| ty.0.clone()))

This PR adds a unit test that reproduces the issue, and then fixes it.


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

@jdisanti jdisanti requested a review from a team as a code owner August 21, 2023 23:34
@github-actions
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.

@jdisanti jdisanti merged commit 9570983 into smithy-rs-release-0.56.x Aug 22, 2023
@jdisanti jdisanti deleted the jdisanti-fix-endpoint-url-config branch August 22, 2023 16:35
@rcoh
Copy link
Collaborator

rcoh commented Aug 28, 2023

side note: we may want to add a diff target that captures changes like this

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.

4 participants