blaginin commented on code in PR #16300:
URL: https://github.com/apache/datafusion/pull/16300#discussion_r2136400344


##########
datafusion-cli/src/object_storage.rs:
##########
@@ -105,9 +106,52 @@ pub async fn get_s3_object_store_builder(
         builder = builder.with_allow_http(*allow_http);
     }
 
+    if let Some(skip_signature) = skip_signature {
+        builder = builder.with_skip_signature(*skip_signature);
+    }
+
     Ok(builder)
 }
 
+/// Credentials from the AWS SDK
+struct CredentialsFromConfig {
+    region: Option<String>,
+    credentials: Option<SharedCredentialsProvider>,
+}
+
+impl CredentialsFromConfig {
+    /// Attempt find AWS S3 credentials via the AWS SDK
+    pub async fn try_new() -> Result<Self> {
+        let config = 
aws_config::defaults(BehaviorVersion::latest()).load().await;
+        let region = config.region().map(|r| r.to_string());
+
+        let credentials = config
+            .credentials_provider()
+            .ok_or_else(|| {
+                DataFusionError::ObjectStore(object_store::Error::Generic {
+                    store: "S3",
+                    source: "Failed to get S3 credentials aws_config".into(),
+                })
+            })?
+            .clone();
+
+        // The credential provider is lazy, so it does not fetch credentials
+        // until they are needed. To ensure that the credentials are valid,
+        // we can call `provide_credentials` here.
+        let credentials = match credentials.provide_credentials().await {
+            Ok(_) => Some(credentials),
+            Err(e) => {

Review Comment:
   nit: does it make sense to expect specific errors? For example, for 
`CredentialsError::InvalidConfiguration` or 
`InvalidConfiguration::ProviderTimedOut`, we probably still want to raise since 
those mean the creds _could_ be set?
   



##########
datafusion-cli/Cargo.toml:
##########
@@ -55,6 +55,7 @@ datafusion = { workspace = true, features = [
 dirs = "6.0.0"
 env_logger = { workspace = true }
 futures = { workspace = true }
+log = "0.4.27"

Review Comment:
   given that cli is now part of the workspace, should we do 
   ```suggestion
   log = { workspace = true }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to