Refactor configuration loading to parse profile files only once #2152
Conversation
rcoh
force-pushed
the
config-profile-refactor
branch
from
b905766 to
c0080e0
Compare
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
| /// .load() | ||
| /// .await; | ||
| /// # } | ||
| pub fn profile_name(mut self, profile_name: impl Into<String>) -> Self { |
There was a problem hiding this comment.
It may be worth linking to https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-profiles.html or similar
| /// This **does not set** the HTTP connector used when sending operations. To change that | ||
| /// connector, use [ConfigLoader::http_connector]. | ||
| /// | ||
| /// NOTE: Update the provider config with care because it will override any other settings. |
There was a problem hiding this comment.
Can we be more specific about what this will override? I'm wondering if we should rename ProviderConfig to better highlight the stuff it provides. I'm thinking of something like this:
OsProviderPlatformProviderSystemProvider
| match AppName::new(app_id.to_owned()) { | ||
| Ok(app_name) => Some(app_name), | ||
| Err(err) => { | ||
| tracing::warn!(err = %err, "`sdk-ua-app-id` property `{}` was invalid", app_id); |
There was a problem hiding this comment.
If possible, we should consider the following:
- add a call to action here that tells people how to fix this or stop the warning
- explain to users what value will be used instead
- explain to users how this may affect logging of requests
- explain why you would want to set this
- explain what makes an app ID invalid
We could perhaps link users to docs, but I couldn't find any that described this field.
There was a problem hiding this comment.
%err will include details so this should be OK
| && profile_set.selected_profile() == "default" | ||
| && profile_set.get_profile("default").is_none() | ||
| { | ||
| if profile_set.selected_profile() == "default" && profile_set.get_profile("default").is_none() { | ||
| tracing::debug!("No default profile defined"); | ||
| return Err(ProfileFileError::NoProfilesDefined); |
There was a problem hiding this comment.
Is it possible for a user to have other profiles but they forgot to set the profile name? If so, then I think this error could be improved. It's not so much that no profiles were defined but that the selected profile (default) couldn't be found.
rcoh
force-pushed
the
config-profile-refactor
branch
from
c0080e0 to
02af1b5
Compare
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
Co-authored-by: Zelda Hessler <zhessler@amazon.com>
Co-authored-by: Zelda Hessler <zhessler@amazon.com>
rcoh
force-pushed
the
config-profile-refactor
branch
from
174e066 to
be5b69e
Compare
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
Motivation and Context
Description
Testing
Checklist
CHANGELOG.next.tomlif I made changes to the AWS SDK, generated SDK code, or SDK runtime cratesBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.