-
Notifications
You must be signed in to change notification settings - Fork 197
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
HTTP Connectors Refactor pt.1 - Laying the groundwork #1144
Conversation
update: replace timeout::Settings with TimeoutConfig add: HttpConnector to aws_types::Config and Builder refactor: divide default_provider.rs modules into separate files
A new doc preview is ready to view. |
A new generated diff is ready to view.
|
A new doc preview is ready to view. |
A new generated diff is ready to view.
|
A new doc preview is ready to view. |
A new generated diff is ready to view.
|
A new doc preview is ready to view. |
A new generated diff is ready to view.
|
A new doc preview is ready to view. |
A new generated diff is ready to view.
|
I love the splitting out the modules! I think that's going to make it easier to find things.
I understand why this is needed, but at the same time, I'm slightly concerned about tying Should we have an
It's unfortunate that the options to automate this in rustfmt still haven't been stabilized--perhaps we should opt into them anyway. I want to avoid developers changing import style back and forth as my tendency is to lump them all together alphabetically. I don't have a strong preference on the style used; just that its consistency should be enforced/corrected by a computer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
remove: impl Default for HttpConnector
A new generated diff is ready to view. |
A new doc preview is ready to view. |
A new generated diff is ready to view.
|
A new doc preview is ready to view. |
A new doc preview is ready to view. |
A new generated diff is ready to view.
|
A new generated diff is ready to view.
|
A new doc preview is ready to view. |
A new generated diff is ready to view.
|
A new generated diff is ready to view.
|
A new doc preview is ready to view. |
A new doc preview is ready to view. |
A new generated diff is ready to view.
|
Motivation and Context
This is the first PR in a series that will change the way that HTTP connector are created, configured, and shared throughout the SDK. The goal is providing a better experience for users that want to easily configure various aspects of how the SDK makes HTTP requests.
Because this is such a large PR, here's a high-level breakdown of the changes:
default_providers
module has be broken up into multiple files, one per submodule.sts
module has be broken up into multiple files, one per submodule.aws_config::connector
module now has it's own file.http_provider
module has been renamed tohttp_credential_provider
to better reflect the fact that it contains theHttpCredentialProvider
and related code.aws_config
modules have be separated into separate groups (libstd imports, same-crate imports, aws and smithy runtime crates). This is an unspoken pattern that we've been following elsewhere.aws_smithy_client::timeout::Settings
struct has been fully replaced byaws_smithy_types::timeout::TimeoutConfig
. I should have done this when I originally added timeout support but better late than never, right? All structs and functions that referencedaws_smithy_client::timeout::Settings
have been updated.MakeConnectorFn
,HttpConnector
, andHttpSettings
have been moved fromaws_config::provider_config
toaws_smithy_client::http_connector
. This is in preparation for a later PR that will change how connectors are created and configured. Also, I had to update the way thatHttpSetting
is created because it's a non-exhaustive struct.HttpConnector::make_connector
has been renamed toHttpConnector::connector
in adherence with how we tend to name the methods on providers that return the thing the provider provides. I'm open to workshopping this name or even reverting this change if y'all don't think it's helpful.aws_types
now depends onaws_smithy_client
.aws_types::Config
now has a builder + setter method for setting anHttpConnector
. Nothing uses this yet, but it will be used when I do the actual "Config owns Connector" refactor in a later PR. I'm OK with removing this change for now if y'all think it would mislead people.NOTE: This PR breaks our example for setting timeouts, I'll create a PR to the docs repo addressing that once this one is merged. It'll be a one-line change.
The next PR in this series will focus on an RFC for "Config owns Connector" + an implementation.
Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.