Skip to content

Conversation

@lauzadis
Copy link
Member

Plumb client configuration properties through to other client configuration properties

Issue #

#711

Description of changes

When a user specifies an HTTP engine or region in the client configuration, these are not passed through to the CredentialsProvider. This PR passes those configurations to the DefaultCredentialsChainProvider, and also passes region from the DefaultCredentialsChainProvider to the ProfileCredentialsProvider.

Note: if a user specifies a custom CredentialsProvider, they will still need to specify the engine both there and in the general client configuration.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@lauzadis lauzadis requested a review from a team as a code owner October 19, 2022 17:15
@lauzadis lauzadis marked this pull request as draft October 19, 2022 17:16
@lauzadis lauzadis marked this pull request as ready for review October 19, 2022 18:33
Copy link
Contributor

@aajtodd aajtodd left a comment

Choose a reason for hiding this comment

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

Fix and ship.

The codegen diff preview action appears to have a bug in it related to getting the correct version of smithy-kotlin to use. Ignore for now, I'll create an issue for it. (You can also probably change the smithy-kotlin version in gradle.properties to match and it should work again).

private val profileName: String? = null,
private val platformProvider: PlatformProvider = Platform,
httpClientEngine: HttpClientEngine? = null,
region: String? = null,
Copy link
Contributor

Choose a reason for hiding this comment

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

fix: Needs documentation

AWS region to make requests to
""".trimIndent()
propertyType = ClientConfigPropertyType.Required()
order = -1
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as smithy-kotlin, leave yourself some room between the default to allow for insertion of properties in-between these if necessary.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@lauzadis lauzadis merged commit 3e4fced into main Oct 21, 2022
@lauzadis lauzadis deleted the fix-plumb-client-config branch October 21, 2022 17:37
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