Skip to content

Conversation

ianbotsf
Copy link
Contributor

Issue #

Partially addresses aws/aws-sdk-kotlin#1413

Description of changes

This change updates endpoint resolution to favor endpointUrl over endpoint discovery when both are provided/enabled. This allows callers to bypass endpoint discovery when using a custom endpoint.

This change is backported from #1221 (slated for v1.5) but only includes non-breaking changes in codegen.

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

@ianbotsf ianbotsf requested a review from a team as a code owner February 13, 2025 01:08
Copy link

Affected Artifacts

Changed in size
Artifact Pull Request (bytes) Latest Release (bytes) Delta (bytes) Delta (percentage)
aws-signing-default-jvm.jar 53,497 51,946 1,551 2.99%
runtime-core-jvm.jar 825,504 818,814 6,690 0.82%

write("originalEndpoint.uri.copy { host = discoveredHost },")
write("originalEndpoint.headers,")
write("originalEndpoint.attributes,")
// Backported from https://github.com/smithy-lang/smithy-kotlin/pull/1221; replace when merging v1.5 to main
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add this as a subtask to our internal "v1.5 minor version bump" tracker?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this mostly as a hint for the inevitable merge conflict. It's not necessarily a TODO or FIXME because resolving this will have to happen as part of the v1.5 release. I don't think adding this to a tracker is any more valuable then the other merge conflicts we anticipate.

write("originalEndpoint.uri.copy { host = discoveredHost },")
write("originalEndpoint.headers,")
write("originalEndpoint.attributes,")
// Backported from https://github.com/smithy-lang/smithy-kotlin/pull/1221; replace when merging v1.5 to main
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: This could be a TODO or FIXME for discoverability

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Answered in #1240 (comment)

@ianbotsf ianbotsf merged commit c1bcb04 into main Feb 13, 2025
16 checks passed
@ianbotsf ianbotsf deleted the fix-endpointurl-vs-endpointdiscovery branch February 13, 2025 16:38
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