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

store identity resolvers in a map #3363

Merged
merged 5 commits into from
Jan 12, 2024
Merged

store identity resolvers in a map #3363

merged 5 commits into from
Jan 12, 2024

Conversation

rcoh
Copy link
Collaborator

@rcoh rcoh commented Jan 12, 2024

Motivation and Context

Description

Identity resolvers aren't a list—they are a map keyed on AuthSchemeId. This updates the internal representation to match.

Testing

Checklist

  • I have updated CHANGELOG.next.toml if I made changes to the smithy-rs codegen or runtime crates
  • I have updated CHANGELOG.next.toml if I made changes to the AWS SDK, generated SDK code, or SDK runtime crates

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

@rcoh rcoh requested a review from a team as a code owner January 12, 2024 16:59
Copy link
Collaborator

@jdisanti jdisanti left a comment

Choose a reason for hiding this comment

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

LGTM!

/// Macro to define the structs for both `RuntimeComponents` and `RuntimeComponentsBuilder`.
///
/// This is a macro in order to keep the fields consistent between the two, and to automatically
/// update the `merge_from` and `build` methods when new components are added.
///
/// It also facilitates unit testing since the overall mechanism can be unit tested with different
/// fields that are easy to check in tests (testing with real components makes it hard
/// to tell that the correct component was selected when merging builders).
/// fields that are easy to check in tests ().
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// fields that are easy to check in tests ().
/// fields that are easy to check in tests.

@rcoh rcoh force-pushed the runtime-components-authschemes branch from 8f08547 to bd682c0 Compare January 12, 2024 18:31
Copy link

A new generated diff is ready to view.

  • AWS SDK (ignoring whitespace)
  • 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 and others added 3 commits January 12, 2024 15:36
This issue addresses a semver compatibility problem similar to the one
described in #3318, except for the
`aws_smithy_http::operation::Metadata` type.

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
When we released smithy-rs release-2023-12-08, we introduced a silent
failure for endpoint prefixes, and not in the newly released version,
but in the previous releases. There is a subtle issue with semver that
causes this. This PR addresses the endpoint prefix part of this problem.
Other PRs will fix other parts that are broken by this semver issue.

The issue is that unstable (0.x) runtime crates are declaring types that
get placed into the `ConfigBag`, and these types are referenced in the
`ConfigBag` across crate boundaries. This by itself isn't a problem, but
because our stable 1.x crates depend on the unstable crates, it becomes
a problem. By releasing 1.1.0 that depends on 0.61, consumers of 1.x
pull in both 0.60 and 0.61. The generated code pulls in 0.60, and the
1.1.x crates pull in 0.61. This is fine since two semver-incompatible
crate versions can be in the dependency closure. Thus, the generated
code which is using 0.60 is placing a 0.60 type in the `ConfigBag`, and
the runtime crates that pull the type out of the `ConfigBag` are
expecting a 0.61 version of it. This leads to the type not existing upon
lookup, which then causes the silent break.

This PR fixes this by moving the `EndpointPrefix` type and its
associated methods into `aws-smithy-runtime-api`, a stable crate. The
`aws-smithy-http` unstable crate is updated to point to the new stable
version so that a patch release of that crate will solve the issue for
old versions going forward as well.

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
@rcoh rcoh requested a review from a team as a code owner January 12, 2024 20:36
@rcoh rcoh enabled auto-merge January 12, 2024 20:37
Copy link

A new generated diff is ready to view.

  • AWS SDK (ignoring whitespace)
  • 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.

@rcoh rcoh added this pull request to the merge queue Jan 12, 2024
Merged via the queue into main with commit 336243c Jan 12, 2024
41 checks passed
@rcoh rcoh deleted the runtime-components-authschemes branch January 12, 2024 21:28
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.

2 participants