plusplusjiajia commented on code in PR #2651:
URL: https://github.com/apache/iceberg-rust/pull/2651#discussion_r4400728825


##########
crates/catalog/rest/src/catalog.rs:
##########
@@ -1465,8 +1484,10 @@ impl SessionCatalog for RestSessionCatalog {
             }
         };
 
+        // The commit response carries no credentials, so build a plain FileIO;
+        // the transaction layer reuses the credentialed one it already holds.
         let file_io = self
-            .load_file_io(Some(&response.metadata_location), None)
+            .load_file_io(Some(&response.metadata_location), None, None)

Review Comment:
   Update: I've since taken the `Transaction` change out of this PR to keep it 
within storage, so the table `Transaction::commit` returns doesn't carry the 
vended credentials yet. I'll fix that in a follow-up.



##########
crates/catalog/rest/src/catalog.rs:
##########
@@ -991,7 +1007,9 @@ impl Catalog for RestCatalog {
             "Metadata location missing in `register_table` response!",
         ))?;
 
-        let file_io = self.load_file_io(Some(metadata_location), None).await?;
+        let file_io = self
+            .load_file_io(Some(metadata_location), None, None)

Review Comment:
   Correction: I was wrong here. The register response is a `LoadTableResult`, 
so it can carry `storage-credentials`. `register_table` now passes them, along 
with the response `config`, to the table's `FileIO`.



-- 
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]

Reply via email to