DerGut commented on code in PR #2920:
URL: https://github.com/apache/iceberg-rust/pull/2920#discussion_r3722313850
##########
crates/catalog/rest/src/catalog.rs:
##########
@@ -60,105 +60,56 @@ const CARGO_PKG_VERSION: &str = env!("CARGO_PKG_VERSION");
const PATH_V1: &str = "v1";
/// Builder for [`RestCatalog`].
-#[derive(Debug)]
+#[derive(Debug, Default)]
pub struct RestCatalogBuilder {
- config: RestCatalogConfig,
- storage_factory: Option<Arc<dyn StorageFactory>>,
- kms_client_factory: Option<Arc<dyn KmsClientFactory>>,
- runtime: Option<Runtime>,
-}
-
-impl Default for RestCatalogBuilder {
- fn default() -> Self {
- Self {
- config: RestCatalogConfig {
- name: None,
- uri: "".to_string(),
- warehouse: None,
- props: HashMap::new(),
- client: None,
- },
- storage_factory: None,
- kms_client_factory: None,
- runtime: None,
- }
- }
+ session: Option<SessionContext>,
+ inner: RestSessionCatalogBuilder,
}
impl CatalogBuilder for RestCatalogBuilder {
type C = RestCatalog;
fn with_storage_factory(mut self, storage_factory: Arc<dyn
StorageFactory>) -> Self {
- self.storage_factory = Some(storage_factory);
+ self.inner = self.inner.with_storage_factory(storage_factory);
self
}
fn with_kms_client_factory(mut self, kms_client_factory: Arc<dyn
KmsClientFactory>) -> Self {
- self.kms_client_factory = Some(kms_client_factory);
+ self.inner = self.inner.with_kms_client_factory(kms_client_factory);
self
}
fn with_runtime(mut self, runtime: Runtime) -> Self {
- self.runtime = Some(runtime);
+ self.inner = self.inner.with_runtime(runtime);
self
}
fn load(
- mut self,
+ self,
name: impl Into<String>,
props: HashMap<String, String>,
) -> impl Future<Output = Result<Self::C>> + Send {
- self.config.name = Some(name.into());
-
- if props.contains_key(REST_CATALOG_PROP_URI) {
- self.config.uri = props
- .get(REST_CATALOG_PROP_URI)
- .cloned()
- .unwrap_or_default();
- }
-
- if props.contains_key(REST_CATALOG_PROP_WAREHOUSE) {
- self.config.warehouse =
props.get(REST_CATALOG_PROP_WAREHOUSE).cloned()
- }
-
- // Collect other remaining properties
- self.config.props = props
- .into_iter()
- .filter(|(k, _)| k != REST_CATALOG_PROP_URI && k !=
REST_CATALOG_PROP_WAREHOUSE)
- .collect();
-
+ let name = name.into();
async move {
- if self.config.name.is_none() {
- Err(Error::new(
- ErrorKind::DataInvalid,
- "Catalog name is required",
- ))
- } else if self.config.uri.is_empty() {
- Err(Error::new(
- ErrorKind::DataInvalid,
- "Catalog uri is required",
- ))
- } else {
- let runtime = self.runtime.unwrap_or_else(Runtime::current);
- let kms_client = match self.kms_client_factory {
- Some(factory) =>
Some(factory.create_kms_client(&self.config.props).await?),
- None => None,
- };
- Ok(RestCatalog::new(
- self.config,
- self.storage_factory,
- runtime,
- kms_client,
- ))
- }
+ let session = self.session.unwrap_or_else(SessionContext::empty);
+ let session_catalog = Arc::new(self.inner.load(name,
props).await?);
+
+ Ok(RestCatalog::from_session_catalog(session, session_catalog))
}
}
}
impl RestCatalogBuilder {
/// Configures the catalog with a custom HTTP client.
pub fn with_client(mut self, client: Client) -> Self {
- self.config.client = Some(client);
+ self.inner = self.inner.with_client(client);
+ self
+ }
+
+ /// Configures the session that will be used with this catalog.
+ /// Overwrites the default empty session from SessionContext::empty().
+ pub fn with_session(mut self, session: SessionContext) -> Self {
Review Comment:
As mentioned in the "Welcome Feedback ❓" section, I could use some opinions
on the `session` vs. `context`/ `ctx` variable naming. I feel like what we
introduce here will be reused throughout.
```suggestion
pub fn with_session_context(mut self, ctx: SessionContext) -> Self {
```
--
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]
