liurenjie1024 commented on issue #1254: URL: https://github.com/apache/iceberg-rust/issues/1254#issuecomment-2829286621
> For `CatalogLoader` trait, how about we have a `CatalogConfig` like: > > pub struct CatalogConfig { > name: String, > uri: String, > warehouse: String, > props: HashMap<String, String>, > ...others > } > So that our CatalogLoader just need to be like: > > pub trait CatalogLoader { > fn load(cfg: CatalogConfig) -> Result<Arc<dyn Catalog>>; > } > After this change, our loader itself is dyn compatible. Users who want to rest specific APIs can still use `RestCatalogBuilder`. > > This change also makes it much easier for pyiceberg to connect since they can pass a `CatalogConfig` to rust directly without extra wrapper code. > > What do you think? Sounds like a solution. Would you mind to create an example pr like https://github.com/apache/iceberg-rust/pull/1231 so that we have a better understand what it would look like? -- 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