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

Reply via email to