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

aws_util: Prepare for supporting S3 as well #5319

Merged
merged 5 commits into from Jan 14, 2021

Conversation

quodlibetor
Copy link
Contributor

@quodlibetor quodlibetor commented Jan 14, 2021

This is a collection of small changes to aws_util that was split out of #5194 / #5202 , which contain further changes to these files.


This change is Reviewable

This will allow it to be shared between different services in later commits.
Before this credentials weren't fetched until we tried to use the client,
causing credentials errors to show up well after the dataflow had been created.
With this we can fail to create the source at all.
Copy link
Contributor

@JLDLaughlin JLDLaughlin left a comment

Choose a reason for hiding this comment

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

looks great to me!

pub access_key_id: Option<String>,
pub secret_access_key: Option<String>,
pub token: Option<String>,
pub aws_info: AwsConnectInfo,
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

@@ -44,6 +46,7 @@ pub async fn client(
info!("AWS access_key_id and secret_access_key not provided, creating a new Kinesis client using a chain provider.");
let mut provider = ChainProvider::new();
provider.set_timeout(Duration::from_secs(10));
provider.credentials().await?; // ensure that credentials exist
Copy link
Contributor

Choose a reason for hiding this comment

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

Great catch!

///
/// The AutoRefreshingProvider caches the underlying provider's AWS credentials,
/// If statically provided connection information information is not provided,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe: If static connection information is not provided, ...

@quodlibetor quodlibetor merged commit ec48c94 into MaterializeInc:main Jan 14, 2021
@quodlibetor quodlibetor deleted the aws-util-unification branch March 10, 2021 20:45
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.

None yet

2 participants