From 57acc063c38657de08f9ebea593eb8e5416db844 Mon Sep 17 00:00:00 2001 From: Ion Koutsouris <15728914+ion-elgreco@users.noreply.github.com> Date: Thu, 22 Aug 2024 15:12:10 +0200 Subject: [PATCH] chore: clean up docs etc --- crates/aws/src/lib.rs | 24 ++++++++------------ crates/aws/src/logstore/default_logstore.rs | 14 ++++++------ crates/aws/src/logstore/dynamodb_logstore.rs | 10 -------- crates/aws/src/logstore/mod.rs | 13 +++++++++-- crates/aws/tests/integration_s3_dynamodb.rs | 2 +- 5 files changed, 28 insertions(+), 35 deletions(-) diff --git a/crates/aws/src/lib.rs b/crates/aws/src/lib.rs index 271ec0387e..1fa4414d06 100644 --- a/crates/aws/src/lib.rs +++ b/crates/aws/src/lib.rs @@ -54,28 +54,22 @@ impl LogStoreFactory for S3LogStoreFactory { .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(logstore::default_logstore::default_s3_logstore( - store, location, options, - )); + return Ok(logstore::default_s3_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(logstore::default_logstore::default_s3_logstore( - store, location, options, - )); + return Ok(logstore::default_s3_logstore(store, location, options)); } - Ok(Arc::new( - logstore::dynamodb_logstore::S3DynamoDbLogStore::try_new( - location.clone(), - options.clone(), - &s3_options, - store, - )?, - )) + Ok(Arc::new(logstore::S3DynamoDbLogStore::try_new( + location.clone(), + options.clone(), + &s3_options, + store, + )?)) } } @@ -734,7 +728,7 @@ mod tests { let logstore = factory .with_options(Arc::new(store), &url, &StorageOptions::from(HashMap::new())) .unwrap(); - assert_eq!(logstore.name(), "DefaultLogStore"); + assert_eq!(logstore.name(), "S3LogStore"); } #[test] diff --git a/crates/aws/src/logstore/default_logstore.rs b/crates/aws/src/logstore/default_logstore.rs index 79bd53f811..a5688141c2 100644 --- a/crates/aws/src/logstore/default_logstore.rs +++ b/crates/aws/src/logstore/default_logstore.rs @@ -15,13 +15,13 @@ use deltalake_core::{ use object_store::{Error as ObjectStoreError, ObjectStore}; use url::Url; -/// Return the [DefaultLogStore] implementation with the provided configuration options +/// Return the [S3LogStore] implementation with the provided configuration options pub fn default_s3_logstore( store: ObjectStoreRef, location: &Url, options: &StorageOptions, ) -> Arc { - Arc::new(DefaultS3LogStore::new( + Arc::new(S3LogStore::new( store, LogStoreConfig { location: location.clone(), @@ -32,13 +32,13 @@ pub fn default_s3_logstore( /// Default [`LogStore`] implementation #[derive(Debug, Clone)] -pub struct DefaultS3LogStore { +pub struct S3LogStore { pub(crate) storage: Arc, config: LogStoreConfig, } -impl DefaultS3LogStore { - /// Create a new instance of [`DefaultLogStore`] +impl S3LogStore { + /// Create a new instance of [`S3LogStore`] /// /// # Arguments /// @@ -50,9 +50,9 @@ impl DefaultS3LogStore { } #[async_trait::async_trait] -impl LogStore for DefaultS3LogStore { +impl LogStore for S3LogStore { fn name(&self) -> String { - "DefaultS3LogStore".into() + "S3LogStore".into() } async fn read_commit_entry(&self, version: i64) -> DeltaResult> { diff --git a/crates/aws/src/logstore/dynamodb_logstore.rs b/crates/aws/src/logstore/dynamodb_logstore.rs index 37a7ed0047..5307040538 100644 --- a/crates/aws/src/logstore/dynamodb_logstore.rs +++ b/crates/aws/src/logstore/dynamodb_logstore.rs @@ -317,13 +317,3 @@ pub enum RepairLogEntryResult { /// Both parts of the repair process where already carried. AlreadyCompleted, } - -/// Represents the possible, positive outcomes of calling `DynamoDbClient::try_create_lock_table()` -#[derive(Debug, PartialEq)] -pub enum CreateLockTableResult { - /// Table created successfully. - TableCreated, - /// Table was not created because it already exists. - /// Does not imply that the table has the correct schema. - TableAlreadyExists, -} diff --git a/crates/aws/src/logstore/mod.rs b/crates/aws/src/logstore/mod.rs index 38c82e0365..e5d7f87aec 100644 --- a/crates/aws/src/logstore/mod.rs +++ b/crates/aws/src/logstore/mod.rs @@ -1,2 +1,11 @@ -pub mod default_logstore; -pub mod dynamodb_logstore; +//! Contains the different logstore implementations for S3. +//! - S3LogStore (used when copy-if-not-exists or unsafe_rename is passed) +//! - S3DynamoDBLogStore (used when DynamoDB is the locking client) + +mod default_logstore; +mod dynamodb_logstore; + +pub use default_logstore::default_s3_logstore; +pub use default_logstore::S3LogStore; +pub use dynamodb_logstore::RepairLogEntryResult; +pub use dynamodb_logstore::S3DynamoDbLogStore; diff --git a/crates/aws/tests/integration_s3_dynamodb.rs b/crates/aws/tests/integration_s3_dynamodb.rs index 3bae9d566b..b933735b4f 100644 --- a/crates/aws/tests/integration_s3_dynamodb.rs +++ b/crates/aws/tests/integration_s3_dynamodb.rs @@ -6,7 +6,7 @@ use std::collections::HashMap; use std::time::{Duration, SystemTime, UNIX_EPOCH}; use aws_sdk_dynamodb::types::BillingMode; -use deltalake_aws::dynamodb_logstore::{RepairLogEntryResult, S3DynamoDbLogStore}; +use deltalake_aws::logstore::{RepairLogEntryResult, S3DynamoDbLogStore}; use deltalake_aws::storage::S3StorageOptions; use deltalake_aws::{CommitEntry, DynamoDbConfig, DynamoDbLockClient}; use deltalake_core::kernel::{Action, Add, DataType, PrimitiveType, StructField, StructType};