[DISCUSS] A catalog loader api.

[liurenjie1024]

When developing iceberg-playground and the integration test framework, I realized that it would be convenient to have a catalog loader api. I have following design in mind:

pub trait CatalogBuilder {
   type C: Catalog;
   fn with_name(self, name: &str) -> Self;
   fn with_uri(self, uri: &str) -> Self;
   fn with_warehouse(self, warehouse: &str) -> Self;
   fn with_prop(self, key: &str, value: &str) -> Self;
   fn build(self) -> Result<Self::C>;
}

And we need to have all catalogs to implement this trait:

pub struct RestCatalogBuilder {
  ...
}

impl CatalogBuilder for RestCatalogBuilder {
  ....
}

impl RestCatalogBuilder {
  fn with_rest_client(self, client: Client) -> Self {
  ...
 }
}

Then we will have a catalog loader api like this:

fn load_catalog(r#type: &str) -> CatalogBuilder {
  match r#type {
     "rest" => RestCatalogBuilder::new()
     "hive" => ....
  }
}

[liurenjie1024]

cc @Xuanwo @sdd @ZENOTME @c-thiel @Fokko

[ZENOTME]

Thanks @liurenjie1024 for raising this discussion. According to my understanding, the CatalogBuilder is used to let us have a unified way to create catalog like following, right?

let catalog_builder = load_catalog(xxx);
catalog_builder.with_name()
               .with_xxx()
               ...
let catalog = catalog_builder.build()

One thing I'm concerned about is that there may be a specific parameter to create the catalog, e.g. S3Catalog and GlueCatalog have different catalog configs. In this case, looks like it's hard to unify the with_xxx of them?

[Xuanwo]

I share the same concern as @ZENOTME.

For example, the REST Catalog supports setting an HttpClient. How can we enable CatalogBuilder to support this as well? Alternatively, should we treat CatalogBuilder as an entry point, and recommend that advanced users—such as those who need to configure the HttpClient—use RestCatalogBuilder directly?

[liurenjie1024]

One thing I'm concerned about is that there may be a specific parameter to create the catalog, e.g. S3Catalog and GlueCatalog have different catalog configs. In this case, looks like it's hard to unify the with_xxx of them?

This configuration could all be configured by keys. For example, table_bucket_arn in S3Catalog could be configured with a key like s3.catalog.table.bucket.arn.

For example, the REST Catalog supports setting an HttpClient. How can we enable CatalogBuilder to support this as well? Alternatively, should we treat CatalogBuilder as an entry point, and recommend that advanced users—such as those who need to configure the HttpClient—use RestCatalogBuilder directly?

Yeah, I totally agree with it. The load_catalog api is to allow user to have a convenient way to creat catalog, but advanced user should still use concrete catalog builder directly.

[kevinjqliu]

+1 this is similar to pyiceberg's load_catalog, i like it

[liurenjie1024]

One thing also worth discussion is whether we need to remove obsolete builder api, for example RestCatalogConfigBuilder. I prefer to remove them, but it's a breaking api change.

[liurenjie1024]

Meeting conclusion: it would be fine to break the obsolete config builder api as we are not 1.0, and we need to provide docs/release notes about the migration.

[gsoundar]

@liurenjie1024 For the load_catalog function, how do you plan to avoid cyclic dependency ? Catalog does not include packages of specific implementations.

I can help with this API. Trying to see what you are going for.

[liurenjie1024]

Hi, @gsoundar I currently have a prototype in #1231 . Per offline discussion with @Xuanwo he has some different opinions and planning to refine the proposal. We can continue discussion in #1231