chore: fix many warnings across the codebase, removed several non-pub…

…lic methods that weren't being used anywhere Signed-off-by: Stephen Carman <[email protected]>
delta-io · Oct 25, 2024 · a676640 · a676640
1 parent e19e21f
commit a676640
Show file tree

Hide file tree

Showing 20 changed files with 144 additions and 681 deletions.
diff --git a/crates/aws/src/constants.rs b/crates/aws/src/constants.rs
@@ -8,6 +8,7 @@ use std::time::Duration;
 /// Custom S3 endpoint.
 pub const AWS_ENDPOINT_URL: &str = "AWS_ENDPOINT_URL";
 /// Custom DynamoDB endpoint.
+///
 /// If DynamoDB endpoint is not supplied, will use S3 endpoint (AWS_ENDPOINT_URL)
 /// If it is supplied, this endpoint takes precedence over the global endpoint set in AWS_ENDPOINT_URL for DynamoDB
 pub const AWS_ENDPOINT_URL_DYNAMODB: &str = "AWS_ENDPOINT_URL_DYNAMODB";
@@ -41,7 +42,9 @@ pub const AWS_IAM_ROLE_SESSION_NAME: &str = "AWS_IAM_ROLE_SESSION_NAME";
     note = "Please use AWS_IAM_ROLE_SESSION_NAME instead"
 )]
 pub const AWS_S3_ROLE_SESSION_NAME: &str = "AWS_S3_ROLE_SESSION_NAME";
-/// The `pool_idle_timeout` option of aws http client. Has to be lower than 20 seconds, which is
+/// The `pool_idle_timeout` option of aws http client.
+///
+/// Has to be lower than 20 seconds, which is
 /// default S3 server timeout <https://aws.amazon.com/premiumsupport/knowledge-center/s3-socket-connection-timeout-error/>.
 /// However, since rusoto uses hyper as a client, its default timeout is 90 seconds
 /// <https://docs.rs/hyper/0.13.2/hyper/client/struct.Builder.html#method.keep_alive_timeout>.
@@ -55,16 +58,19 @@ pub const AWS_STS_POOL_IDLE_TIMEOUT_SECONDS: &str = "AWS_STS_POOL_IDLE_TIMEOUT_S
 pub const AWS_S3_GET_INTERNAL_SERVER_ERROR_RETRIES: &str =
     "AWS_S3_GET_INTERNAL_SERVER_ERROR_RETRIES";
 /// The web identity token file to use when using a web identity provider.
+///
 /// NOTE: web identity related options are set in the environment when
 /// creating an instance of [crate::storage::s3::S3StorageOptions].
 /// See also <https://docs.rs/rusoto_sts/0.47.0/rusoto_sts/struct.WebIdentityProvider.html#method.from_k8s_env>.
 pub const AWS_WEB_IDENTITY_TOKEN_FILE: &str = "AWS_WEB_IDENTITY_TOKEN_FILE";
 /// The role name to use for web identity.
+///
 /// NOTE: web identity related options are set in the environment when
 /// creating an instance of [crate::storage::s3::S3StorageOptions].
 /// See also <https://docs.rs/rusoto_sts/0.47.0/rusoto_sts/struct.WebIdentityProvider.html#method.from_k8s_env>.
 pub const AWS_ROLE_ARN: &str = "AWS_ROLE_ARN";
 /// The role session name to use for web identity.
+///
 /// NOTE: web identity related options are set in the environment when
 /// creating an instance of [crate::storage::s3::S3StorageOptions].
 /// See also <https://docs.rs/rusoto_sts/0.47.0/rusoto_sts/struct.WebIdentityProvider.html#method.from_k8s_env>.
@@ -99,8 +105,8 @@ pub const S3_OPTS: &[&str] = &[
     AWS_SECRET_ACCESS_KEY,
     AWS_SESSION_TOKEN,
     AWS_S3_LOCKING_PROVIDER,
-    AWS_S3_ASSUME_ROLE_ARN,
-    AWS_S3_ROLE_SESSION_NAME,
+    AWS_IAM_ROLE_ARN,
+    AWS_IAM_ROLE_SESSION_NAME,
     AWS_WEB_IDENTITY_TOKEN_FILE,
     AWS_ROLE_ARN,
     AWS_ROLE_SESSION_NAME,

diff --git a/crates/aws/src/credentials.rs b/crates/aws/src/credentials.rs
@@ -21,7 +21,7 @@ use deltalake_core::storage::StorageOptions;
 use deltalake_core::DeltaResult;
 use tracing::log::*;
 
-use crate::constants::{self, AWS_ENDPOINT_URL};
+use crate::constants;
 
 /// An [object_store::CredentialProvider] which handles converting a populated [SdkConfig]
 /// into a necessary [AwsCredential] type for configuring [object_store::aws::AmazonS3]
@@ -183,19 +183,26 @@ fn assume_role_arn(options: &StorageOptions) -> Option<String> {
     options
         .0
         .get(constants::AWS_IAM_ROLE_ARN)
-        .or(options.0.get(constants::AWS_S3_ASSUME_ROLE_ARN))
+        .or(
+            #[allow(deprecated)]
+            options.0.get(constants::AWS_S3_ASSUME_ROLE_ARN),
+        )
         .or(std::env::var_os(constants::AWS_IAM_ROLE_ARN)
             .map(|o| {
                 o.into_string()
                     .expect("Failed to unwrap AWS_IAM_ROLE_ARN which may have invalid data")
             })
             .as_ref())
-        .or(std::env::var_os(constants::AWS_S3_ASSUME_ROLE_ARN)
-            .map(|o| {
-                o.into_string()
-                    .expect("Failed to unwrap AWS_S3_ASSUME_ROLE_ARN which may have invalid data")
-            })
-            .as_ref())
+        .or(
+            #[allow(deprecated)]
+            std::env::var_os(constants::AWS_S3_ASSUME_ROLE_ARN)
+                .map(|o| {
+                    o.into_string().expect(
+                        "Failed to unwrap AWS_S3_ASSUME_ROLE_ARN which may have invalid data",
+                    )
+                })
+                .as_ref(),
+        )
         .cloned()
 }
 
@@ -204,13 +211,13 @@ fn assume_session_name(options: &StorageOptions) -> String {
     let assume_session = options
         .0
         .get(constants::AWS_IAM_ROLE_SESSION_NAME)
-        .or(options.0.get(constants::AWS_S3_ROLE_SESSION_NAME))
+        .or(
+            #[allow(deprecated)]
+            options.0.get(constants::AWS_S3_ROLE_SESSION_NAME),
+        )
         .cloned();
 
-    match assume_session {
-        Some(s) => s,
-        None => assume_role_sessio_name(),
-    }
+    assume_session.unwrap_or_else(assume_role_sessio_name)
 }
 
 /// Take a set of [StorageOptions] and produce an appropriate AWS SDK [SdkConfig]

diff --git a/crates/aws/src/lib.rs b/crates/aws/src/lib.rs
@@ -57,7 +57,7 @@ impl LogStoreFactory for S3LogStoreFactory {
         // With conditional put in S3-like API we can use the deltalake default logstore which use PutIfAbsent
         if options.0.keys().any(|key| {
             let key = key.to_ascii_lowercase();
-            vec![
+            [
                 AmazonS3ConfigKey::ConditionalPut.as_ref(),
                 "conditional_put",
             ]
@@ -69,7 +69,7 @@ impl LogStoreFactory for S3LogStoreFactory {
 
         if options.0.keys().any(|key| {
             let key = key.to_ascii_lowercase();
-            vec![
+            [
                 AmazonS3ConfigKey::CopyIfNotExists.as_ref(),
                 "copy_if_not_exists",
             ]
@@ -306,9 +306,11 @@ impl DynamoDbLockClient {
                         .send()
                         .await
                 },
-                |err| match err.as_service_error() {
-                    Some(GetItemError::ProvisionedThroughputExceededException(_)) => true,
-                    _ => false,
+                |err| {
+                    matches!(
+                        err.as_service_error(),
+                        Some(GetItemError::ProvisionedThroughputExceededException(_))
+                    )
                 },
             )
             .await
@@ -340,9 +342,11 @@ impl DynamoDbLockClient {
                     .await?;
                 Ok(())
             },
-            |err: &SdkError<_, _>| match err.as_service_error() {
-                Some(PutItemError::ProvisionedThroughputExceededException(_)) => true,
-                _ => false,
+            |err: &SdkError<_, _>| {
+                matches!(
+                    err.as_service_error(),
+                    Some(PutItemError::ProvisionedThroughputExceededException(_))
+                )
             },
         )
         .await
@@ -395,9 +399,11 @@ impl DynamoDbLockClient {
                         .send()
                         .await
                 },
-                |err: &SdkError<_, _>| match err.as_service_error() {
-                    Some(QueryError::ProvisionedThroughputExceededException(_)) => true,
-                    _ => false,
+                |err: &SdkError<_, _>| {
+                    matches!(
+                        err.as_service_error(),
+                        Some(QueryError::ProvisionedThroughputExceededException(_))
+                    )
                 },
             )
             .await
@@ -446,9 +452,11 @@ impl DynamoDbLockClient {
                         .await?;
                     Ok(())
                 },
-                |err: &SdkError<_, _>| match err.as_service_error() {
-                    Some(UpdateItemError::ProvisionedThroughputExceededException(_)) => true,
-                    _ => false,
+                |err: &SdkError<_, _>| {
+                    matches!(
+                        err.as_service_error(),
+                        Some(UpdateItemError::ProvisionedThroughputExceededException(_))
+                    )
                 },
             )
             .await;
@@ -488,9 +496,11 @@ impl DynamoDbLockClient {
                     .await?;
                 Ok(())
             },
-            |err: &SdkError<_, _>| match err.as_service_error() {
-                Some(DeleteItemError::ProvisionedThroughputExceededException(_)) => true,
-                _ => false,
+            |err: &SdkError<_, _>| {
+                matches!(
+                    err.as_service_error(),
+                    Some(DeleteItemError::ProvisionedThroughputExceededException(_))
+                )
             },
         )
         .await