liurenjie1024 commented on code in PR #1523: URL: https://github.com/apache/iceberg-rust/pull/1523#discussion_r2222120319
########## crates/iceberg/src/spec/table_metadata.rs: ########## @@ -464,6 +465,26 @@ impl TableMetadata { self.encryption_keys.get(key_id) } + /// Read table metadata from the given location. + pub async fn read(file_io: &FileIO, metadata_location: &str) -> Result<TableMetadata> { + let input_file = file_io.new_input(metadata_location)?; + let metadata_content = input_file.read().await?; + let metadata = serde_json::from_slice::<TableMetadata>(&metadata_content)?; + Ok(metadata) + } + + /// Write table metadata to the given location. + pub async fn write( + file_io: &FileIO, + metadata: &TableMetadata, Review Comment: nit: This could be an instance method? e.g. `write(&self, ...)` ########## crates/catalog/glue/src/catalog.rs: ########## @@ -395,10 +395,7 @@ impl Catalog for GlueCatalog { .metadata; let metadata_location = create_metadata_location(&location, 0)?; - self.file_io - .new_output(&metadata_location)? - .write(serde_json::to_vec(&metadata)?.into()) - .await?; + TableMetadata::write(&self.file_io, &metadata, &metadata_location).await?; Review Comment: I think this should not change the interface here. We could change the write interface to sth like following: ```rust fn write(&self, impl ToString) ``` to generalize the arguments. -- 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