liurenjie1024 commented on code in PR #1372:
URL: https://github.com/apache/iceberg-rust/pull/1372#discussion_r2106849970
##########
crates/iceberg/src/catalog/mod.rs:
##########
@@ -36,7 +37,19 @@ use crate::spec::{
use crate::table::Table;
use crate::{Error, ErrorKind, Result};
+/// The CatalogLoader trait is used to load a catalog from a given name and
properties.
+#[async_trait]
+pub trait CatalogLoader: Debug + Send + Sync {
+ /// Load a catalog from the given name and properties.
+ async fn load(properties: HashMap<String, String>) -> Result<Arc<dyn
Catalog>>;
Review Comment:
There are two problems with this approach:
1. This is not easy to use when we know the concret type of catalog. For
example, when the user just wants to create a RestCatalog. It will force user
to do downcast. This is useful when the catalog has some extran functionality.
2. This is not easy to use when the catalog builder has an advanced builder
method, see https://github.com/apache/iceberg-rust/pull/1231/files#r2106848332
I think I can simplify the methods in my original proposal like your one,
while keeping other things same, WDYT?
##########
crates/iceberg/src/catalog/mod.rs:
##########
@@ -36,7 +37,19 @@ use crate::spec::{
use crate::table::Table;
use crate::{Error, ErrorKind, Result};
+/// The CatalogLoader trait is used to load a catalog from a given name and
properties.
+#[async_trait]
+pub trait CatalogLoader: Debug + Send + Sync {
+ /// Load a catalog from the given name and properties.
+ async fn load(properties: HashMap<String, String>) -> Result<Arc<dyn
Catalog>>;
Review Comment:
```suggestion
async fn load(name: String, properties: HashMap<String, String>) ->
Result<Arc<dyn Catalog>>;
```
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
