CTTY commented on code in PR #2080:
URL: https://github.com/apache/iceberg-rust/pull/2080#discussion_r2752491522
##########
crates/iceberg/src/io/storage.rs:
##########
@@ -154,41 +157,150 @@ pub trait StorageFactory: Debug + Send + Sync {
fn build(&self, config: &StorageConfig) -> Result<Arc<dyn Storage>>;
}
-/// The storage carries all supported storage services in iceberg
-#[derive(Debug)]
-pub(crate) enum OpenDalStorage {
+/// OpenDAL-based storage factory.
+///
+/// Maps scheme to the corresponding OpenDalStorage storage variant.
+///
+/// TODO this is currently not used, we still use OpenDalStorage::build() for
now
+#[derive(Clone, Debug, Serialize, Deserialize)]
+pub enum OpenDalStorageFactory {
Review Comment:
This sounds good, I've moved opendal storage and related functions to a new
module `opendal`
##########
crates/iceberg/src/io/storage.rs:
##########
@@ -154,41 +157,150 @@ pub trait StorageFactory: Debug + Send + Sync {
fn build(&self, config: &StorageConfig) -> Result<Arc<dyn Storage>>;
}
-/// The storage carries all supported storage services in iceberg
-#[derive(Debug)]
-pub(crate) enum OpenDalStorage {
+/// OpenDAL-based storage factory.
+///
+/// Maps scheme to the corresponding OpenDalStorage storage variant.
+///
+/// TODO this is currently not used, we still use OpenDalStorage::build() for
now
+#[derive(Clone, Debug, Serialize, Deserialize)]
+pub enum OpenDalStorageFactory {
+ /// Memory storage factory.
#[cfg(feature = "storage-memory")]
- Memory(Operator),
+ Memory,
+ /// Local filesystem storage factory.
+ #[cfg(feature = "storage-fs")]
+ Fs,
+ /// S3 storage factory.
+ #[cfg(feature = "storage-s3")]
+ S3 {
+ /// Custom AWS credential loader.
+ #[serde(skip)]
+ customized_credential_load: Option<CustomAwsCredentialLoader>,
+ },
+ /// GCS storage factory.
+ #[cfg(feature = "storage-gcs")]
+ Gcs,
+ /// OSS storage factory.
+ #[cfg(feature = "storage-oss")]
+ Oss,
+ /// Azure Data Lake Storage factory.
+ #[cfg(feature = "storage-azdls")]
+ Azdls {
+ /// The configured Azure storage scheme.
+ configured_scheme: AzureStorageScheme,
+ },
+}
+
+#[typetag::serde(name = "OpenDalStorageFactory")]
+impl StorageFactory for OpenDalStorageFactory {
+ #[allow(unused_variables)]
+ fn build(&self, config: &StorageConfig) -> Result<Arc<dyn Storage>> {
+ match self {
+ #[cfg(feature = "storage-memory")]
+ OpenDalStorageFactory::Memory =>
Ok(Arc::new(OpenDalStorage::Memory(
+ super::memory_config_build()?,
+ ))),
+ #[cfg(feature = "storage-fs")]
+ OpenDalStorageFactory::Fs => Ok(Arc::new(OpenDalStorage::LocalFs)),
+ #[cfg(feature = "storage-s3")]
+ OpenDalStorageFactory::S3 {
+ customized_credential_load,
+ } => Ok(Arc::new(OpenDalStorage::S3 {
+ configured_scheme: "s3".to_string(),
+ config: super::s3_config_parse(config.props().clone())?.into(),
+ customized_credential_load: customized_credential_load.clone(),
+ })),
+ #[cfg(feature = "storage-gcs")]
+ OpenDalStorageFactory::Gcs => Ok(Arc::new(OpenDalStorage::Gcs {
+ config:
super::gcs_config_parse(config.props().clone())?.into(),
+ })),
+ #[cfg(feature = "storage-oss")]
+ OpenDalStorageFactory::Oss => Ok(Arc::new(OpenDalStorage::Oss {
+ config:
super::oss_config_parse(config.props().clone())?.into(),
+ })),
+ #[cfg(feature = "storage-azdls")]
+ OpenDalStorageFactory::Azdls { configured_scheme } => {
+ Ok(Arc::new(OpenDalStorage::Azdls {
+ configured_scheme: configured_scheme.clone(),
+ config:
super::azdls_config_parse(config.props().clone())?.into(),
+ }))
+ }
+ #[cfg(all(
+ not(feature = "storage-memory"),
+ not(feature = "storage-fs"),
+ not(feature = "storage-s3"),
+ not(feature = "storage-gcs"),
+ not(feature = "storage-oss"),
+ not(feature = "storage-azdls"),
+ ))]
+ _ => Err(Error::new(
+ ErrorKind::FeatureUnsupported,
+ "No storage service has been enabled",
+ )),
+ }
+ }
+}
+
+/// Default memory operator for serde deserialization.
+#[cfg(feature = "storage-memory")]
+fn default_memory_operator() -> Operator {
+ super::memory_config_build().expect("Failed to create default memory
operator")
Review Comment:
Added an ut
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
