check via the config key rather than S3StorageOptions

delta-io · Jan 16, 2024 · a97e9a6 · a97e9a6
1 parent 13d02f3
commit a97e9a6
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 11 deletions.
diff --git a/crates/deltalake-aws/src/lib.rs b/crates/deltalake-aws/src/lib.rs
@@ -5,6 +5,7 @@ pub mod logstore;
 pub mod storage;
 
 use lazy_static::lazy_static;
+use object_store::aws::AmazonS3ConfigKey;
 use regex::Regex;
 use std::{
     collections::HashMap,
@@ -41,15 +42,19 @@ impl LogStoreFactory for S3LogStoreFactory {
         options: &StorageOptions,
     ) -> DeltaResult<Arc<dyn LogStore>> {
         let store = url_prefix_handler(store, Path::parse(location.path())?)?;
-        let s3_options = S3StorageOptions::from_map(&options.0);
 
-        if s3_options.copy_if_not_exists.is_some() {
+        if options
+            .0
+            .contains_key(AmazonS3ConfigKey::CopyIfNotExists.as_ref())
+        {
             debug!("S3LogStoreFactory has been asked to create a LogStore where the underlying store has copy-if-not-exists enabled - no locking provider required");
             return Ok(deltalake_core::logstore::default_logstore(
                 store, location, options,
             ));
         }
 
+        let s3_options = S3StorageOptions::from_map(&options.0);
+
         if s3_options.locking_provider.as_deref() != Some("dynamodb") {
             debug!("S3LogStoreFactory has been asked to create a LogStore without the dynamodb locking provider");
             return Ok(deltalake_core::logstore::default_logstore(

diff --git a/crates/deltalake-aws/src/storage.rs b/crates/deltalake-aws/src/storage.rs
@@ -56,11 +56,16 @@ impl ObjectStoreFactory for S3ObjectStoreFactory {
             }),
         )?;
 
-        let options = S3StorageOptions::from_map(&options.0);
-        if options.copy_if_not_exists.is_some() {
+        if options
+            .0
+            .contains_key(AmazonS3ConfigKey::CopyIfNotExists.as_ref())
+        {
             // If the copy-if-not-exists env var is set, we don't need to instantiate a locking client or check for allow-unsafe-rename.
             return Ok((Arc::from(store), prefix));
         }
+
+        let options = S3StorageOptions::from_map(&options.0);
+
         let store = S3StorageBackend::try_new(
             store.into(),
             Some("dynamodb") == options.locking_provider.as_deref() || options.allow_unsafe_rename,
@@ -91,7 +96,6 @@ pub struct S3StorageOptions {
     pub sts_pool_idle_timeout: Duration,
     pub s3_get_internal_server_error_retries: usize,
     pub allow_unsafe_rename: bool,
-    pub copy_if_not_exists: Option<String>,
     pub extra_opts: HashMap<String, String>,
 }
 
@@ -146,8 +150,6 @@ impl S3StorageOptions {
             .map(|val| str_is_truthy(&val))
             .unwrap_or(false);
 
-        let copy_if_not_exists = str_option(options, s3_constants::AWS_COPY_IF_NOT_EXISTS);
-
         Self {
             endpoint_url,
             region,
@@ -165,7 +167,6 @@ impl S3StorageOptions {
             s3_get_internal_server_error_retries,
             allow_unsafe_rename,
             extra_opts,
-            copy_if_not_exists,
         }
     }
 
@@ -454,7 +455,6 @@ mod tests {
                 s3_get_internal_server_error_retries: 10,
                 extra_opts: HashMap::new(),
                 allow_unsafe_rename: false,
-                copy_if_not_exists: None
             },
             options
         );
@@ -524,7 +524,6 @@ mod tests {
                     s3_constants::AWS_S3_ADDRESSING_STYLE.to_string() => "virtual".to_string()
                 },
                 allow_unsafe_rename: false,
-                copy_if_not_exists: None
             },
             options
         );
@@ -577,7 +576,6 @@ mod tests {
                 s3_get_internal_server_error_retries: 3,
                 extra_opts: hashmap! {},
                 allow_unsafe_rename: false,
-                copy_if_not_exists: None
             },
             options
         );