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

Clarify that non-aws-config based config Builders don't have good defaults #1226

Open
Mark-Simulacrum opened this issue Dec 20, 2024 · 2 comments
Labels
documentation This is a problem with documentation needs-triage This issue or PR still needs to be triaged.

Comments

@Mark-Simulacrum
Copy link

Describe the issue

Customers using e.g. aws_sdk_sts::config::Builder::new() and customizing a few settings (e.g., credential provider) don't currently receive defaults for other fields, such as the retry policy.

Ideally we'd have strong warnings against usage of these methods in favor of the aws-config constructors, and potentially consider some stronger messaging (e.g., deprecating in favor of without_defaults() or similarly less "easy to use" methods). Note that there's a bunch of ways to get such a builder today:

Each should likely have similar treatment. The docs on the Config struct also say:

The service config can also be constructed manually using its builder.

Which probably should be clarified to say "and this is almost certainly not what you want".

Links

(Also present in every other SDK):

@Mark-Simulacrum Mark-Simulacrum added documentation This is a problem with documentation needs-triage This issue or PR still needs to be triaged. labels Dec 20, 2024
github-merge-queue bot pushed a commit to smithy-lang/smithy-rs that referenced this issue Jan 3, 2025
## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here -->
Partially addresses awslabs/aws-sdk-rust#1226,
although we might want to look into going further and actually
deprecating or warning on these builder methods.

## Description
<!--- Describe your changes in detail -->

## Testing
<!--- Please describe in detail how you tested your changes -->
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->
Doc only update

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
@landonxjames
Copy link
Contributor

Added a warning to those methods in smithy-lang/smithy-rs#3960, but we will also discuss marking them as deprecated going forward. Leaving this issue open pending those discussions.

@Mark-Simulacrum
Copy link
Author

https://docs.rs/aws-config/latest/aws_config/imds/client/struct.Builder.html might also be an interesting case of a special client that seems like it can't be created from the aws-config Config? (That might make sense, since there's recursive dependency implied there for at least some of the configuration).

It might be a good idea to update its documentation if it's Builder is special and does come with the recommended defaults -- that looks true to me from a quick glance at the code, but don't trust me on that :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation This is a problem with documentation needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

No branches or pull requests

2 participants