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


##########
crates/catalog/rest/src/catalog.rs:
##########
@@ -1076,6 +1074,23 @@ impl Catalog for RestCatalog {
     }
 }
 
+/// FileIO props: server `config`, then vended `storage_credentials` (longest 
prefix wins), then user props.
+fn table_file_io_config(

Review Comment:
   I also notice that Java constructs `clientByPrefix` 
[Link](https://github.com/apache/iceberg/blob/main/aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIO.java#L405)
 which keeps credentials separate per prefix. So we can't support 
multi-location tables here I think? Also it would silently fail in those cases 
which I don't think is ideal? WDYT?



##########
crates/catalog/rest/src/catalog.rs:
##########
@@ -825,6 +822,8 @@ impl Catalog for RestCatalog {
         let request = context
             .client
             .request(Method::GET, context.config.table_endpoint(table_ident))
+            // Opt in to vended storage credentials.
+            .header("X-Iceberg-Access-Delegation", "vended-credentials")

Review Comment:
   Java will read these headers off the catalog config, should we do the same? 
   ```
   header.X-Iceberg-Access-Delegation
   ```
   [Link to 
Java](https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/rest/RESTUtil.java#L378)



##########
crates/catalog/rest/src/catalog.rs:
##########
@@ -1062,9 +1057,12 @@ impl Catalog for RestCatalog {
             }
         };
 
+        // Reload for a credentialed FileIO (commit response carries no 
credentials).
         let file_io = self
-            .load_file_io(Some(&response.metadata_location), None)
-            .await?;
+            .load_table(commit.identifier())

Review Comment:
   A full load table seems heavy to me. Can we not wire in the pre-credentialed 
FileIO into this method somehow? 



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