From 1ecac849beb19c4f80802a917030c8003e92d718 Mon Sep 17 00:00:00 2001 From: Leon Lin Date: Sat, 16 Aug 2025 23:23:21 -0700 Subject: [PATCH 01/10] Add hms catalog loader implementation --- Cargo.lock | 1 + Cargo.toml | 1 + crates/catalog/hms/src/catalog.rs | 104 ++++++++++++++++++- crates/catalog/hms/tests/hms_catalog_test.rs | 31 ++++-- crates/catalog/loader/Cargo.toml | 1 + crates/catalog/loader/src/lib.rs | 27 +++++ 6 files changed, 151 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a0b74a8786..56e41a2686 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3613,6 +3613,7 @@ version = "0.6.0" dependencies = [ "async-trait", "iceberg", + "iceberg-catalog-hms", "iceberg-catalog-rest", "tokio", ] diff --git a/Cargo.toml b/Cargo.toml index a585be7d77..ec7ec06332 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -77,6 +77,7 @@ hive_metastore = "0.1" http = "1.2" iceberg = { version = "0.6.0", path = "./crates/iceberg" } iceberg-catalog-rest = { version = "0.6.0", path = "./crates/catalog/rest" } +iceberg-catalog-hms = { version = "0.6.0", path = "./crates/catalog/hms" } iceberg-datafusion = { version = "0.6.0", path = "./crates/integrations/datafusion" } indicatif = "0.17" itertools = "0.13" diff --git a/crates/catalog/hms/src/catalog.rs b/crates/catalog/hms/src/catalog.rs index 899811f969..c50248aadb 100644 --- a/crates/catalog/hms/src/catalog.rs +++ b/crates/catalog/hms/src/catalog.rs @@ -29,8 +29,8 @@ use iceberg::io::FileIO; use iceberg::spec::{TableMetadata, TableMetadataBuilder}; use iceberg::table::Table; use iceberg::{ - Catalog, Error, ErrorKind, MetadataLocation, Namespace, NamespaceIdent, Result, TableCommit, - TableCreation, TableIdent, + Catalog, CatalogBuilder, Error, ErrorKind, MetadataLocation, Namespace, NamespaceIdent, Result, + TableCommit, TableCreation, TableIdent, }; use typed_builder::TypedBuilder; use volo_thrift::MaybeException; @@ -38,6 +38,101 @@ use volo_thrift::MaybeException; use super::utils::*; use crate::error::{from_io_error, from_thrift_error, from_thrift_exception}; +/// HMS catalog address +pub const HMS_CATALOG_PROP_ADDRESS: &str = "address"; + +/// HMS Catalog thrift transport +pub const HMS_CATALOG_PROP_THRIFT_TRANSPORT: &str = "thrift_transport"; +/// HMS Catalog framed thrift transport +pub const THRIFT_TRANSPORT_FRAMED: &str = "framed"; +/// HMS Catalog framed buffered transport +pub const THRIFT_TRANSPORT_BUFFERED: &str = "buffered"; + +/// HMS Catalog warehouse location +pub const HMS_CATALOG_PROP_WAREHOUSE: &str = "warehouse"; + +/// Builder for [`RestCatalog`]. +#[derive(Debug)] +pub struct HmsCatalogBuilder(HmsCatalogConfig); + +impl Default for HmsCatalogBuilder { + fn default() -> Self { + Self(HmsCatalogConfig { + name: None, + address: "".to_string(), + thrift_transport: HmsThriftTransport::default(), + warehouse: "".to_string(), + props: HashMap::new(), + }) + } +} + +impl CatalogBuilder for HmsCatalogBuilder { + type C = HmsCatalog; + + fn load( + mut self, + name: impl Into, + props: HashMap, + ) -> impl Future> + Send { + self.0.name = Some(name.into()); + + if props.contains_key(HMS_CATALOG_PROP_ADDRESS) { + self.0.address = props + .get(HMS_CATALOG_PROP_ADDRESS) + .cloned() + .unwrap_or_default(); + } + + if let Some(tt) = props.get(HMS_CATALOG_PROP_THRIFT_TRANSPORT) { + self.0.thrift_transport = match tt.to_lowercase().as_str() { + THRIFT_TRANSPORT_FRAMED => HmsThriftTransport::Framed, + THRIFT_TRANSPORT_BUFFERED => HmsThriftTransport::Buffered, + _ => HmsThriftTransport::default(), + }; + } + + if props.contains_key(HMS_CATALOG_PROP_WAREHOUSE) { + self.0.warehouse = props + .get(HMS_CATALOG_PROP_WAREHOUSE) + .cloned() + .unwrap_or_default(); + } + + self.0.props = props + .into_iter() + .filter(|(k, _)| { + k != HMS_CATALOG_PROP_ADDRESS + && k != HMS_CATALOG_PROP_THRIFT_TRANSPORT + && k != HMS_CATALOG_PROP_WAREHOUSE + }) + .collect(); + + let result = { + if self.0.name.is_none() { + Err(Error::new( + ErrorKind::DataInvalid, + "Catalog name is required", + )) + } else if self.0.address.is_empty() { + Err(Error::new( + ErrorKind::DataInvalid, + "Catalog address is required", + )) + } else if self.0.warehouse.is_empty() { + Err(Error::new( + ErrorKind::DataInvalid, + "Catalog warehouse is required", + )) + } else { + HmsCatalog::new(self.0) + } + }; + + std::future::ready(result) + } +} + /// Which variant of the thrift transport to communicate with HMS /// See: #[derive(Debug, Default)] @@ -51,7 +146,8 @@ pub enum HmsThriftTransport { /// Hive metastore Catalog configuration. #[derive(Debug, TypedBuilder)] -pub struct HmsCatalogConfig { +pub(crate) struct HmsCatalogConfig { + name: Option, address: String, thrift_transport: HmsThriftTransport, warehouse: String, @@ -78,7 +174,7 @@ impl Debug for HmsCatalog { impl HmsCatalog { /// Create a new hms catalog. - pub fn new(config: HmsCatalogConfig) -> Result { + fn new(config: HmsCatalogConfig) -> Result { let address = config .address .as_str() diff --git a/crates/catalog/hms/tests/hms_catalog_test.rs b/crates/catalog/hms/tests/hms_catalog_test.rs index 74f1e3c14e..c99cb3ca56 100644 --- a/crates/catalog/hms/tests/hms_catalog_test.rs +++ b/crates/catalog/hms/tests/hms_catalog_test.rs @@ -24,8 +24,11 @@ use std::sync::RwLock; use ctor::{ctor, dtor}; use iceberg::io::{S3_ACCESS_KEY_ID, S3_ENDPOINT, S3_REGION, S3_SECRET_ACCESS_KEY}; use iceberg::spec::{NestedField, PrimitiveType, Schema, Type}; -use iceberg::{Catalog, Namespace, NamespaceIdent, TableCreation, TableIdent}; -use iceberg_catalog_hms::{HmsCatalog, HmsCatalogConfig, HmsThriftTransport}; +use iceberg::{Catalog, CatalogBuilder, Namespace, NamespaceIdent, TableCreation, TableIdent}; +use iceberg_catalog_hms::{ + HMS_CATALOG_PROP_ADDRESS, HMS_CATALOG_PROP_THRIFT_TRANSPORT, HMS_CATALOG_PROP_WAREHOUSE, + HmsCatalog, HmsCatalogBuilder, THRIFT_TRANSPORT_BUFFERED, +}; use iceberg_test_utils::docker::DockerCompose; use iceberg_test_utils::{normalize_test_name, set_up}; use port_scanner::scan_port_addr; @@ -79,6 +82,18 @@ async fn get_catalog() -> HmsCatalog { } let props = HashMap::from([ + ( + HMS_CATALOG_PROP_ADDRESS.to_string(), + hms_socket_addr.to_string(), + ), + ( + HMS_CATALOG_PROP_THRIFT_TRANSPORT.to_string(), + THRIFT_TRANSPORT_BUFFERED.to_string(), + ), + ( + HMS_CATALOG_PROP_WAREHOUSE.to_string(), + "s3a://warehouse/hive".to_string(), + ), ( S3_ENDPOINT.to_string(), format!("http://{}", minio_socket_addr), @@ -106,14 +121,10 @@ async fn get_catalog() -> HmsCatalog { retries += 1; } - let config = HmsCatalogConfig::builder() - .address(hms_socket_addr.to_string()) - .thrift_transport(HmsThriftTransport::Buffered) - .warehouse("s3a://warehouse/hive".to_string()) - .props(props) - .build(); - - HmsCatalog::new(config).unwrap() + HmsCatalogBuilder::default() + .load("hms", props) + .await + .unwrap() } async fn set_test_namespace(catalog: &HmsCatalog, namespace: &NamespaceIdent) -> Result<()> { diff --git a/crates/catalog/loader/Cargo.toml b/crates/catalog/loader/Cargo.toml index d29edad051..ed6790a0ce 100644 --- a/crates/catalog/loader/Cargo.toml +++ b/crates/catalog/loader/Cargo.toml @@ -31,5 +31,6 @@ repository = { workspace = true } [dependencies] iceberg = { workspace = true } iceberg-catalog-rest = {workspace = true} +iceberg-catalog-hms = { workspace = true } tokio = { workspace = true } async-trait = {workspace = true} diff --git a/crates/catalog/loader/src/lib.rs b/crates/catalog/loader/src/lib.rs index e5fce46822..6be99b6a7a 100644 --- a/crates/catalog/loader/src/lib.rs +++ b/crates/catalog/loader/src/lib.rs @@ -20,6 +20,7 @@ use std::sync::Arc; use async_trait::async_trait; use iceberg::{Catalog, CatalogBuilder, Error, ErrorKind, Result}; +use iceberg_catalog_hms::HmsCatalogBuilder; use iceberg_catalog_rest::RestCatalogBuilder; #[async_trait] @@ -46,6 +47,7 @@ impl BoxedCatalogBuilder for T { pub fn load(r#type: &str) -> Result> { match r#type { "rest" => Ok(Box::new(RestCatalogBuilder::default()) as Box), + "hms" => Ok(Box::new(HmsCatalogBuilder::default()) as Box), _ => Err(Error::new( ErrorKind::FeatureUnsupported, format!("Unsupported catalog type: {}", r#type), @@ -79,4 +81,29 @@ mod tests { assert!(catalog.is_ok()); } + + #[tokio::test] + async fn test_load_hms_catalog() { + use iceberg_catalog_hms::{HMS_CATALOG_PROP_ADDRESS, HMS_CATALOG_PROP_WAREHOUSE}; + + let catalog_loader = load("hms").unwrap(); + let catalog = catalog_loader + .load( + "hms".to_string(), + HashMap::from([ + ( + HMS_CATALOG_PROP_ADDRESS.to_string(), + "127.0.0.1:1".to_string(), + ), + ( + HMS_CATALOG_PROP_WAREHOUSE.to_string(), + "s3://warehouse".to_string(), + ), + ("key".to_string(), "value".to_string()), + ]), + ) + .await; + + assert!(catalog.is_ok()); + } } From 669eff9fa1fd7e6271fe30e203a25c7dcf29c7bc Mon Sep 17 00:00:00 2001 From: Leon Lin Date: Sun, 17 Aug 2025 11:07:54 -0700 Subject: [PATCH 02/10] add documentation --- crates/catalog/hms/src/catalog.rs | 2 +- crates/catalog/hms/src/lib.rs | 32 +++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/crates/catalog/hms/src/catalog.rs b/crates/catalog/hms/src/catalog.rs index c50248aadb..34b5f679f1 100644 --- a/crates/catalog/hms/src/catalog.rs +++ b/crates/catalog/hms/src/catalog.rs @@ -45,7 +45,7 @@ pub const HMS_CATALOG_PROP_ADDRESS: &str = "address"; pub const HMS_CATALOG_PROP_THRIFT_TRANSPORT: &str = "thrift_transport"; /// HMS Catalog framed thrift transport pub const THRIFT_TRANSPORT_FRAMED: &str = "framed"; -/// HMS Catalog framed buffered transport +/// HMS Catalog buffered thrift transport pub const THRIFT_TRANSPORT_BUFFERED: &str = "buffered"; /// HMS Catalog warehouse location diff --git a/crates/catalog/hms/src/lib.rs b/crates/catalog/hms/src/lib.rs index db0034d46b..c830da8f54 100644 --- a/crates/catalog/hms/src/lib.rs +++ b/crates/catalog/hms/src/lib.rs @@ -16,6 +16,38 @@ // under the License. //! Iceberg Hive Metastore Catalog implementation. +//! +//! To build a hive metastore with configurations +//! # Example +//! +//! ```rust, no_run +//! use std::collections::HashMap; +//! +//! use iceberg::CatalogBuilder; +//! use iceberg_catalog_hms::{ +//! HMS_CATALOG_PROP_ADDRESS, HMS_CATALOG_PROP_WAREHOUSE, HmsCatalogBuilder, +//! }; +//! +//! #[tokio::main] +//! async fn main() { +//! let catalog = HmsCatalogBuilder::default() +//! .load( +//! "hms", +//! HashMap::from([ +//! ( +//! HMS_CATALOG_PROP_ADDRESS.to_string(), +//! "127.0.0.1:1".to_string(), +//! ), +//! ( +//! HMS_CATALOG_PROP_WAREHOUSE.to_string(), +//! "s3://warehouse".to_string(), +//! ), +//! ]), +//! ) +//! .await +//! .unwrap(); +//! } +//! ``` #![deny(missing_docs)] From f191ffb5d8ce18e0b8d7f5d3b2ea0f0fc575dee1 Mon Sep 17 00:00:00 2001 From: Leon Lin Date: Wed, 20 Aug 2025 15:40:24 -0700 Subject: [PATCH 03/10] Update crates/catalog/hms/src/catalog.rs Co-authored-by: Renjie Liu --- crates/catalog/hms/src/catalog.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/catalog/hms/src/catalog.rs b/crates/catalog/hms/src/catalog.rs index 34b5f679f1..f6fec6f7fb 100644 --- a/crates/catalog/hms/src/catalog.rs +++ b/crates/catalog/hms/src/catalog.rs @@ -39,7 +39,7 @@ use super::utils::*; use crate::error::{from_io_error, from_thrift_error, from_thrift_exception}; /// HMS catalog address -pub const HMS_CATALOG_PROP_ADDRESS: &str = "address"; +pub const HMS_CATALOG_PROP_URI: &str = "uri"; /// HMS Catalog thrift transport pub const HMS_CATALOG_PROP_THRIFT_TRANSPORT: &str = "thrift_transport"; From e7f3602d3753c8077cf37447f3097bd68c723b3f Mon Sep 17 00:00:00 2001 From: Leon Lin Date: Wed, 20 Aug 2025 15:50:03 -0700 Subject: [PATCH 04/10] update configs accordingly --- crates/catalog/hms/src/catalog.rs | 6 +++--- crates/catalog/hms/tests/hms_catalog_test.rs | 4 ++-- crates/catalog/loader/src/lib.rs | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/catalog/hms/src/catalog.rs b/crates/catalog/hms/src/catalog.rs index f6fec6f7fb..c9f1e4477b 100644 --- a/crates/catalog/hms/src/catalog.rs +++ b/crates/catalog/hms/src/catalog.rs @@ -77,9 +77,9 @@ impl CatalogBuilder for HmsCatalogBuilder { ) -> impl Future> + Send { self.0.name = Some(name.into()); - if props.contains_key(HMS_CATALOG_PROP_ADDRESS) { + if props.contains_key(HMS_CATALOG_PROP_URI) { self.0.address = props - .get(HMS_CATALOG_PROP_ADDRESS) + .get(HMS_CATALOG_PROP_URI) .cloned() .unwrap_or_default(); } @@ -102,7 +102,7 @@ impl CatalogBuilder for HmsCatalogBuilder { self.0.props = props .into_iter() .filter(|(k, _)| { - k != HMS_CATALOG_PROP_ADDRESS + k != HMS_CATALOG_PROP_URI && k != HMS_CATALOG_PROP_THRIFT_TRANSPORT && k != HMS_CATALOG_PROP_WAREHOUSE }) diff --git a/crates/catalog/hms/tests/hms_catalog_test.rs b/crates/catalog/hms/tests/hms_catalog_test.rs index c99cb3ca56..e2398144db 100644 --- a/crates/catalog/hms/tests/hms_catalog_test.rs +++ b/crates/catalog/hms/tests/hms_catalog_test.rs @@ -26,7 +26,7 @@ use iceberg::io::{S3_ACCESS_KEY_ID, S3_ENDPOINT, S3_REGION, S3_SECRET_ACCESS_KEY use iceberg::spec::{NestedField, PrimitiveType, Schema, Type}; use iceberg::{Catalog, CatalogBuilder, Namespace, NamespaceIdent, TableCreation, TableIdent}; use iceberg_catalog_hms::{ - HMS_CATALOG_PROP_ADDRESS, HMS_CATALOG_PROP_THRIFT_TRANSPORT, HMS_CATALOG_PROP_WAREHOUSE, + HMS_CATALOG_PROP_URI, HMS_CATALOG_PROP_THRIFT_TRANSPORT, HMS_CATALOG_PROP_WAREHOUSE, HmsCatalog, HmsCatalogBuilder, THRIFT_TRANSPORT_BUFFERED, }; use iceberg_test_utils::docker::DockerCompose; @@ -83,7 +83,7 @@ async fn get_catalog() -> HmsCatalog { let props = HashMap::from([ ( - HMS_CATALOG_PROP_ADDRESS.to_string(), + HMS_CATALOG_PROP_URI.to_string(), hms_socket_addr.to_string(), ), ( diff --git a/crates/catalog/loader/src/lib.rs b/crates/catalog/loader/src/lib.rs index 6be99b6a7a..8ae262c969 100644 --- a/crates/catalog/loader/src/lib.rs +++ b/crates/catalog/loader/src/lib.rs @@ -84,7 +84,7 @@ mod tests { #[tokio::test] async fn test_load_hms_catalog() { - use iceberg_catalog_hms::{HMS_CATALOG_PROP_ADDRESS, HMS_CATALOG_PROP_WAREHOUSE}; + use iceberg_catalog_hms::{HMS_CATALOG_PROP_URI, HMS_CATALOG_PROP_WAREHOUSE}; let catalog_loader = load("hms").unwrap(); let catalog = catalog_loader @@ -92,7 +92,7 @@ mod tests { "hms".to_string(), HashMap::from([ ( - HMS_CATALOG_PROP_ADDRESS.to_string(), + HMS_CATALOG_PROP_URI.to_string(), "127.0.0.1:1".to_string(), ), ( From 8bdec07788d70e2c78fb2915fedc7e5ad151ed35 Mon Sep 17 00:00:00 2001 From: Leon Lin Date: Wed, 20 Aug 2025 15:56:49 -0700 Subject: [PATCH 05/10] Update loader --- Cargo.lock | 1 + crates/catalog/loader/src/lib.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 48a995242f..66c76d8cf6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3595,6 +3595,7 @@ dependencies = [ "async-trait", "iceberg", "iceberg-catalog-glue", + "iceberg-catalog-hms", "iceberg-catalog-rest", "iceberg-catalog-s3tables", "tokio", diff --git a/crates/catalog/loader/src/lib.rs b/crates/catalog/loader/src/lib.rs index d1b80307ed..a105ba527b 100644 --- a/crates/catalog/loader/src/lib.rs +++ b/crates/catalog/loader/src/lib.rs @@ -33,6 +33,7 @@ static CATALOG_REGISTRY: &[(&str, CatalogBuilderFactory)] = &[ ("rest", || Box::new(RestCatalogBuilder::default())), ("glue", || Box::new(GlueCatalogBuilder::default())), ("s3tables", || Box::new(S3TablesCatalogBuilder::default())), + ("hms", || Box::new(HmsCatalogBuilder::default())), ]; /// Return the list of supported catalog types. From 0e4cb2f3b25a620d7e0e863765701d057d7b2ff3 Mon Sep 17 00:00:00 2001 From: Leon Lin Date: Wed, 20 Aug 2025 15:59:06 -0700 Subject: [PATCH 06/10] remove type builder and fix formatting --- Cargo.lock | 1 - crates/catalog/hms/Cargo.toml | 1 - crates/catalog/hms/src/catalog.rs | 9 ++------- crates/catalog/hms/tests/hms_catalog_test.rs | 2 +- crates/catalog/loader/src/lib.rs | 5 +---- 5 files changed, 4 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 66c76d8cf6..5b3ba0df36 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3583,7 +3583,6 @@ dependencies = [ "serde_json", "tokio", "tracing", - "typed-builder 0.20.1", "volo", "volo-thrift", ] diff --git a/crates/catalog/hms/Cargo.toml b/crates/catalog/hms/Cargo.toml index 707f3ed6a4..549dbb9c02 100644 --- a/crates/catalog/hms/Cargo.toml +++ b/crates/catalog/hms/Cargo.toml @@ -38,7 +38,6 @@ pilota = { workspace = true } serde_json = { workspace = true } tokio = { workspace = true } tracing = { workspace = true } -typed-builder = { workspace = true } volo-thrift = { workspace = true } # Transitive dependencies below diff --git a/crates/catalog/hms/src/catalog.rs b/crates/catalog/hms/src/catalog.rs index c9f1e4477b..c8f046cb7e 100644 --- a/crates/catalog/hms/src/catalog.rs +++ b/crates/catalog/hms/src/catalog.rs @@ -32,7 +32,6 @@ use iceberg::{ Catalog, CatalogBuilder, Error, ErrorKind, MetadataLocation, Namespace, NamespaceIdent, Result, TableCommit, TableCreation, TableIdent, }; -use typed_builder::TypedBuilder; use volo_thrift::MaybeException; use super::utils::*; @@ -78,10 +77,7 @@ impl CatalogBuilder for HmsCatalogBuilder { self.0.name = Some(name.into()); if props.contains_key(HMS_CATALOG_PROP_URI) { - self.0.address = props - .get(HMS_CATALOG_PROP_URI) - .cloned() - .unwrap_or_default(); + self.0.address = props.get(HMS_CATALOG_PROP_URI).cloned().unwrap_or_default(); } if let Some(tt) = props.get(HMS_CATALOG_PROP_THRIFT_TRANSPORT) { @@ -145,13 +141,12 @@ pub enum HmsThriftTransport { } /// Hive metastore Catalog configuration. -#[derive(Debug, TypedBuilder)] +#[derive(Debug)] pub(crate) struct HmsCatalogConfig { name: Option, address: String, thrift_transport: HmsThriftTransport, warehouse: String, - #[builder(default)] props: HashMap, } diff --git a/crates/catalog/hms/tests/hms_catalog_test.rs b/crates/catalog/hms/tests/hms_catalog_test.rs index e2398144db..2bf12779b1 100644 --- a/crates/catalog/hms/tests/hms_catalog_test.rs +++ b/crates/catalog/hms/tests/hms_catalog_test.rs @@ -26,7 +26,7 @@ use iceberg::io::{S3_ACCESS_KEY_ID, S3_ENDPOINT, S3_REGION, S3_SECRET_ACCESS_KEY use iceberg::spec::{NestedField, PrimitiveType, Schema, Type}; use iceberg::{Catalog, CatalogBuilder, Namespace, NamespaceIdent, TableCreation, TableIdent}; use iceberg_catalog_hms::{ - HMS_CATALOG_PROP_URI, HMS_CATALOG_PROP_THRIFT_TRANSPORT, HMS_CATALOG_PROP_WAREHOUSE, + HMS_CATALOG_PROP_THRIFT_TRANSPORT, HMS_CATALOG_PROP_URI, HMS_CATALOG_PROP_WAREHOUSE, HmsCatalog, HmsCatalogBuilder, THRIFT_TRANSPORT_BUFFERED, }; use iceberg_test_utils::docker::DockerCompose; diff --git a/crates/catalog/loader/src/lib.rs b/crates/catalog/loader/src/lib.rs index a105ba527b..0b3f067e47 100644 --- a/crates/catalog/loader/src/lib.rs +++ b/crates/catalog/loader/src/lib.rs @@ -161,10 +161,7 @@ mod tests { .load( "hms".to_string(), HashMap::from([ - ( - HMS_CATALOG_PROP_URI.to_string(), - "127.0.0.1:1".to_string(), - ), + (HMS_CATALOG_PROP_URI.to_string(), "127.0.0.1:1".to_string()), ( HMS_CATALOG_PROP_WAREHOUSE.to_string(), "s3://warehouse".to_string(), From 8afe61150da1483a852e003100a1c6a35a6340b3 Mon Sep 17 00:00:00 2001 From: Leon Lin Date: Wed, 20 Aug 2025 16:00:19 -0700 Subject: [PATCH 07/10] Fix doc --- crates/catalog/hms/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/catalog/hms/src/lib.rs b/crates/catalog/hms/src/lib.rs index c830da8f54..e4250c7507 100644 --- a/crates/catalog/hms/src/lib.rs +++ b/crates/catalog/hms/src/lib.rs @@ -25,7 +25,7 @@ //! //! use iceberg::CatalogBuilder; //! use iceberg_catalog_hms::{ -//! HMS_CATALOG_PROP_ADDRESS, HMS_CATALOG_PROP_WAREHOUSE, HmsCatalogBuilder, +//! HMS_CATALOG_PROP_URI, HMS_CATALOG_PROP_WAREHOUSE, HmsCatalogBuilder, //! }; //! //! #[tokio::main] @@ -35,7 +35,7 @@ //! "hms", //! HashMap::from([ //! ( -//! HMS_CATALOG_PROP_ADDRESS.to_string(), +//! HMS_CATALOG_PROP_URI.to_string(), //! "127.0.0.1:1".to_string(), //! ), //! ( From 9b37fe4ed9cc0b0e09136cdf30c848db56570d9a Mon Sep 17 00:00:00 2001 From: Leon Lin Date: Wed, 20 Aug 2025 16:01:44 -0700 Subject: [PATCH 08/10] Fix formatting --- crates/catalog/hms/src/lib.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/catalog/hms/src/lib.rs b/crates/catalog/hms/src/lib.rs index e4250c7507..ea87400f08 100644 --- a/crates/catalog/hms/src/lib.rs +++ b/crates/catalog/hms/src/lib.rs @@ -34,10 +34,7 @@ //! .load( //! "hms", //! HashMap::from([ -//! ( -//! HMS_CATALOG_PROP_URI.to_string(), -//! "127.0.0.1:1".to_string(), -//! ), +//! (HMS_CATALOG_PROP_URI.to_string(), "127.0.0.1:1".to_string()), //! ( //! HMS_CATALOG_PROP_WAREHOUSE.to_string(), //! "s3://warehouse".to_string(), From 9e85ddc76a595ad4de77ce6b88f7df48a34cf1ee Mon Sep 17 00:00:00 2001 From: Leon Lin Date: Wed, 20 Aug 2025 16:39:54 -0700 Subject: [PATCH 09/10] Update test names and retrigger failed flaky tests on CI --- crates/catalog/loader/src/lib.rs | 58 ++++++++++++++++---------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/crates/catalog/loader/src/lib.rs b/crates/catalog/loader/src/lib.rs index 0b3f067e47..3ada2ea496 100644 --- a/crates/catalog/loader/src/lib.rs +++ b/crates/catalog/loader/src/lib.rs @@ -111,7 +111,33 @@ mod tests { use crate::{CatalogLoader, load}; #[tokio::test] - async fn test_load_glue_catalog() { + async fn test_load_unsupported_catalog() { + let result = load("unsupported"); + assert!(result.is_err()); + } + + #[tokio::test] + async fn test_catalog_loader_pattern() { + use iceberg_catalog_rest::REST_CATALOG_PROP_URI; + + let catalog = CatalogLoader::from("rest") + .load( + "rest".to_string(), + HashMap::from([ + ( + REST_CATALOG_PROP_URI.to_string(), + "http://localhost:8080".to_string(), + ), + ("key".to_string(), "value".to_string()), + ]), + ) + .await; + + assert!(catalog.is_ok()); + } + + #[tokio::test] + async fn test_catalog_loader_pattern_glue_catalog() { use iceberg_catalog_glue::GLUE_CATALOG_PROP_WAREHOUSE; let catalog_loader = load("glue").unwrap(); @@ -132,7 +158,7 @@ mod tests { } #[tokio::test] - async fn test_load_rest_catalog() { + async fn test_catalog_loader_pattern_rest_catalog() { use iceberg_catalog_rest::REST_CATALOG_PROP_URI; let catalog_loader = load("rest").unwrap(); @@ -153,7 +179,7 @@ mod tests { } #[tokio::test] - async fn test_load_hms_catalog() { + async fn test_catalog_loader_pattern_hms_catalog() { use iceberg_catalog_hms::{HMS_CATALOG_PROP_URI, HMS_CATALOG_PROP_WAREHOUSE}; let catalog_loader = load("hms").unwrap(); @@ -174,32 +200,6 @@ mod tests { assert!(catalog.is_ok()); } - #[tokio::test] - async fn test_load_unsupported_catalog() { - let result = load("unsupported"); - assert!(result.is_err()); - } - - #[tokio::test] - async fn test_catalog_loader_pattern() { - use iceberg_catalog_rest::REST_CATALOG_PROP_URI; - - let catalog = CatalogLoader::from("rest") - .load( - "rest".to_string(), - HashMap::from([ - ( - REST_CATALOG_PROP_URI.to_string(), - "http://localhost:8080".to_string(), - ), - ("key".to_string(), "value".to_string()), - ]), - ) - .await; - - assert!(catalog.is_ok()); - } - #[tokio::test] async fn test_catalog_loader_pattern_s3tables() { use iceberg_catalog_s3tables::S3TABLES_CATALOG_PROP_TABLE_BUCKET_ARN; From 015643d4b93cf185aa63c6a74144434fb75932ea Mon Sep 17 00:00:00 2001 From: Leon Lin Date: Thu, 21 Aug 2025 00:05:01 -0700 Subject: [PATCH 10/10] reorder tests and restart random cancelled tests --- crates/catalog/loader/src/lib.rs | 52 ++++++++++++++++---------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/crates/catalog/loader/src/lib.rs b/crates/catalog/loader/src/lib.rs index 3ada2ea496..9c18ab4e5f 100644 --- a/crates/catalog/loader/src/lib.rs +++ b/crates/catalog/loader/src/lib.rs @@ -137,17 +137,17 @@ mod tests { } #[tokio::test] - async fn test_catalog_loader_pattern_glue_catalog() { - use iceberg_catalog_glue::GLUE_CATALOG_PROP_WAREHOUSE; + async fn test_catalog_loader_pattern_rest_catalog() { + use iceberg_catalog_rest::REST_CATALOG_PROP_URI; - let catalog_loader = load("glue").unwrap(); + let catalog_loader = load("rest").unwrap(); let catalog = catalog_loader .load( - "glue".to_string(), + "rest".to_string(), HashMap::from([ ( - GLUE_CATALOG_PROP_WAREHOUSE.to_string(), - "s3://test".to_string(), + REST_CATALOG_PROP_URI.to_string(), + "http://localhost:8080".to_string(), ), ("key".to_string(), "value".to_string()), ]), @@ -158,17 +158,17 @@ mod tests { } #[tokio::test] - async fn test_catalog_loader_pattern_rest_catalog() { - use iceberg_catalog_rest::REST_CATALOG_PROP_URI; + async fn test_catalog_loader_pattern_glue_catalog() { + use iceberg_catalog_glue::GLUE_CATALOG_PROP_WAREHOUSE; - let catalog_loader = load("rest").unwrap(); + let catalog_loader = load("glue").unwrap(); let catalog = catalog_loader .load( - "rest".to_string(), + "glue".to_string(), HashMap::from([ ( - REST_CATALOG_PROP_URI.to_string(), - "http://localhost:8080".to_string(), + GLUE_CATALOG_PROP_WAREHOUSE.to_string(), + "s3://test".to_string(), ), ("key".to_string(), "value".to_string()), ]), @@ -179,18 +179,16 @@ mod tests { } #[tokio::test] - async fn test_catalog_loader_pattern_hms_catalog() { - use iceberg_catalog_hms::{HMS_CATALOG_PROP_URI, HMS_CATALOG_PROP_WAREHOUSE}; + async fn test_catalog_loader_pattern_s3tables() { + use iceberg_catalog_s3tables::S3TABLES_CATALOG_PROP_TABLE_BUCKET_ARN; - let catalog_loader = load("hms").unwrap(); - let catalog = catalog_loader + let catalog = CatalogLoader::from("s3tables") .load( - "hms".to_string(), + "s3tables".to_string(), HashMap::from([ - (HMS_CATALOG_PROP_URI.to_string(), "127.0.0.1:1".to_string()), ( - HMS_CATALOG_PROP_WAREHOUSE.to_string(), - "s3://warehouse".to_string(), + S3TABLES_CATALOG_PROP_TABLE_BUCKET_ARN.to_string(), + "arn:aws:s3tables:us-east-1:123456789012:bucket/test".to_string(), ), ("key".to_string(), "value".to_string()), ]), @@ -201,16 +199,18 @@ mod tests { } #[tokio::test] - async fn test_catalog_loader_pattern_s3tables() { - use iceberg_catalog_s3tables::S3TABLES_CATALOG_PROP_TABLE_BUCKET_ARN; + async fn test_catalog_loader_pattern_hms_catalog() { + use iceberg_catalog_hms::{HMS_CATALOG_PROP_URI, HMS_CATALOG_PROP_WAREHOUSE}; - let catalog = CatalogLoader::from("s3tables") + let catalog_loader = load("hms").unwrap(); + let catalog = catalog_loader .load( - "s3tables".to_string(), + "hms".to_string(), HashMap::from([ + (HMS_CATALOG_PROP_URI.to_string(), "127.0.0.1:1".to_string()), ( - S3TABLES_CATALOG_PROP_TABLE_BUCKET_ARN.to_string(), - "arn:aws:s3tables:us-east-1:123456789012:bucket/test".to_string(), + HMS_CATALOG_PROP_WAREHOUSE.to_string(), + "s3://warehouse".to_string(), ), ("key".to_string(), "value".to_string()), ]),