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

pub use aws-sdk client #566

Merged
merged 2 commits into from
Sep 22, 2023
Merged

pub use aws-sdk client #566

merged 2 commits into from
Sep 22, 2023

Conversation

adrian-kong
Copy link
Contributor

@adrian-kong adrian-kong commented Sep 21, 2023

Re-exports aws_sdk_s3

use esthri::aws_sdk::Client;
// use aws_sdk_s3::Client

pub static S3_CLIENT: OnceCell<Client> = OnceCell::const_new();

pub async fn init_s3_client() {
    let client = init_default_s3client().await;
    S3_CLIENT.set(client).unwrap();
}

without pulling in aws_sdk_s3 dependency

  1. Unsure if I've exported all the necessary types - this just exposes StorageClass and Client
  2. This exports as Client instead of S3Client, under esthri::aws_sdk, might be confusing compared to previous versions

@adrian-kong adrian-kong requested a review from a team September 21, 2023 23:48
Copy link
Contributor

@pcrumley pcrumley left a comment

Choose a reason for hiding this comment

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

I don't feel strongly about this so seems reasonable but can exporting this increase likelihood of circular dependencies?

@@ -51,8 +51,6 @@ tokio = { version = "1.23.0", features = ["rt-multi-thread", "signal", "sync"] }
tokio-util = { version = "0.7.8", features = ["compat", "codec"] }
warp = "0.3"

aws-sdk-s3 = { workspace = true }

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand why you would remove this as a workspace dependency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we pull in esthri, we can remove aws-sdk-s3 dependency.
I'm thinking since esthri is tightly coupled with aws-sdk-s3 basically building on top of that, we should maybe export that to tightly couple the version as well for people using this library

@adrian-kong adrian-kong merged commit fee284e into master Sep 22, 2023
@adrian-kong adrian-kong deleted the adrian/re-export-aws-sdk branch September 22, 2023 01:36
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