Merge pull request JanKaul#184 from JanKaul/s3tables-error

make get_table_metadata_location_error transparent

Author: JanKaul

catalogs/iceberg-glue-catalog/src/lib.rs

@@ -55,6 +55,7 @@ pub mod schema;
 mod utils;
 
 impl GlueCatalog {
+    #[allow(clippy::result_large_err)]
     pub fn new(
         config: &SdkConfig,
         name: &str,

catalogs/iceberg-s3tables-catalog/src/error.rs

@@ -4,6 +4,7 @@ use aws_sdk_s3tables::{
     operation::{
         create_namespace::CreateNamespaceError, create_table::CreateTableError,
         delete_table::DeleteTableError, get_table::GetTableError,
+        get_table_metadata_location::GetTableMetadataLocationError,
         list_namespaces::ListNamespacesError, list_tables::ListTablesError,
         update_table_metadata_location::UpdateTableMetadataLocationError,
     },
@@ -26,7 +27,11 @@ pub enum Error {
     #[error(transparent)]
     GetTable(#[from] SdkError<GetTableError, HttpResponse>),
     #[error(transparent)]
-    DeletaTable(#[from] SdkError<DeleteTableError, HttpResponse>),
+    DeleteTable(#[from] SdkError<DeleteTableError, HttpResponse>),
+    #[error(transparent)]
+    SdkError(#[from] SdkError<GetTableMetadataLocationError, HttpResponse>),
+    #[error(transparent)]
+    GetTableMetadataLocation(#[from] GetTableMetadataLocationError),
     #[error(transparent)]
     CreateTable(#[from] SdkError<CreateTableError, HttpResponse>),
     #[error(transparent)]

catalogs/iceberg-s3tables-catalog/src/lib.rs

@@ -6,7 +6,10 @@ use std::{
 use async_trait::async_trait;
 use aws_config::SdkConfig;
 
-use aws_sdk_s3tables::{types::OpenTableFormat, Client};
+use aws_sdk_s3tables::{
+    error::SdkError, operation::get_table_metadata_location::GetTableMetadataLocationError,
+    types::OpenTableFormat, Client,
+};
 use iceberg_rust::{
     catalog::{
         commit::{
@@ -50,6 +53,7 @@ pub struct S3TablesCatalog {
 pub mod error;
 
 impl S3TablesCatalog {
+    #[allow(clippy::result_large_err)]
     pub fn new(
         config: &SdkConfig,
         arn: &str,
@@ -236,7 +240,15 @@ impl Catalog for S3TablesCatalog {
             .name(identifier.name())
             .send()
             .await
-            .map_err(|_| IcebergError::CatalogNotFound)?;
+            .map_err(|err| match err {
+                SdkError::ServiceError(err) => match err.into_err() {
+                    GetTableMetadataLocationError::NotFoundException(_) => {
+                        IcebergError::CatalogNotFound
+                    }
+                    x => Error::from(x).into(),
+                },
+                x => Error::from(x).into(),
+            })?;
 
         let metadata_location = table.metadata_location.ok_or(Error::Text(format!(
             "Table {} not found.",

iceberg-rust-spec/src/error.rs

@@ -27,7 +27,7 @@ pub enum Error {
     NotSupported(String),
     /// Avro error
     #[error(transparent)]
-    Avro(#[from] apache_avro::Error),
+    Avro(Box<apache_avro::Error>),
     /// Serde json
     #[error(transparent)]
     JSONSerde(#[from] serde_json::Error),
@@ -80,3 +80,9 @@ pub enum Error {
     #[error(transparent)]
     PartitionSpec(#[from] crate::spec::partition::PartitionSpecBuilderError),
 }
+
+impl From<apache_avro::Error> for Error {
+    fn from(err: apache_avro::Error) -> Self {
+        Error::Avro(Box::new(err))
+    }
+}

iceberg-rust/src/error.rs

@@ -46,7 +46,7 @@ pub enum Error {
     Parquet(#[from] parquet::errors::ParquetError),
     /// Avro error
     #[error(transparent)]
-    Avro(#[from] apache_avro::Error),
+    Avro(Box<apache_avro::Error>),
     /// Thrift error
     #[error(transparent)]
     Thrift(#[from] thrift::Error),
@@ -115,6 +115,12 @@ pub enum Error {
     ),
 }
 
+impl From<apache_avro::Error> for Error {
+    fn from(err: apache_avro::Error) -> Self {
+        Error::Avro(Box::new(err))
+    }
+}
+
 impl From<Error> for ArrowError {
     fn from(value: Error) -> Self {
         ArrowError::from_external_error(Box::new(value))

iceberg-rust/src/object_store/mod.rs

@@ -64,9 +64,9 @@ impl Bucket<'_> {
 #[derive(Debug, Clone)]
 pub enum ObjectStoreBuilder {
     /// AWS s3 builder
-    S3(AmazonS3Builder),
+    S3(Box<AmazonS3Builder>),
     /// Google Cloud Storage builder
-    GCS(GoogleCloudStorageBuilder),
+    GCS(Box<GoogleCloudStorageBuilder>),
     /// Filesystem builder
     Filesystem(Arc<LocalFileSystem>),
     /// In memory builder
@@ -99,11 +99,11 @@ impl FromStr for ConfigKey {
 impl ObjectStoreBuilder {
     /// Create new AWS S3 Object Store builder
     pub fn s3() -> Self {
-        ObjectStoreBuilder::S3(AmazonS3Builder::from_env())
+        ObjectStoreBuilder::S3(Box::new(AmazonS3Builder::from_env()))
     }
     /// Create new AWS S3 Object Store builder
     pub fn gcs() -> Self {
-        ObjectStoreBuilder::GCS(GoogleCloudStorageBuilder::from_env())
+        ObjectStoreBuilder::GCS(Box::new(GoogleCloudStorageBuilder::from_env()))
     }
     /// Create a new FileSystem ObjectStoreBuilder
     pub fn filesystem(prefix: impl AsRef<Path>) -> Self {
@@ -117,10 +117,10 @@ impl ObjectStoreBuilder {
     pub fn with_config(self, key: ConfigKey, value: impl Into<String>) -> Self {
         match (self, key) {
             (ObjectStoreBuilder::S3(aws), ConfigKey::AWS(key)) => {
-                ObjectStoreBuilder::S3(aws.with_config(key, value))
+                ObjectStoreBuilder::S3(Box::new(aws.with_config(key, value)))
             }
             (ObjectStoreBuilder::GCS(gcs), ConfigKey::GCS(key)) => {
-                ObjectStoreBuilder::GCS(gcs.with_config(key, value))
+                ObjectStoreBuilder::GCS(Box::new(gcs.with_config(key, value)))
             }
             (x, _) => x,
         }
@@ -129,15 +129,15 @@ impl ObjectStoreBuilder {
     pub fn build(&self, bucket: Bucket) -> Result<Arc<dyn ObjectStore>, Error> {
         match (bucket, self) {
             (Bucket::S3(bucket), Self::S3(builder)) => Ok::<_, Error>(Arc::new(
-                builder
+                (**builder)
                     .clone()
                     .with_bucket_name(bucket)
                     .with_copy_if_not_exists(S3CopyIfNotExists::Multipart)
                     .build()
                     .map_err(Error::from)?,
             )),
             (Bucket::GCS(bucket), Self::GCS(builder)) => Ok::<_, Error>(Arc::new(
-                builder
+                (**builder)
                     .clone()
                     .with_bucket_name(bucket)
                     .build()

iceberg-rust/src/sql.rs

@@ -13,7 +13,7 @@ pub fn find_relations(sql: &str) -> Result<Vec<String>, Error> {
     let statements = Parser::parse_sql(&GenericDialect, sql)?;
     let mut visited = Vec::new();
 
-    visit_relations(&statements, |relation| {
+    let _ = visit_relations(&statements, |relation| {
         visited.push(relation.to_string());
         ControlFlow::<()>::Continue(())
     });