danielcweeks commented on code in PR #17709:
URL: https://github.com/apache/iceberg/pull/17709#discussion_r3813913752


##########
core/src/main/java/org/apache/iceberg/rest/RESTTableScan.java:
##########
@@ -228,7 +228,24 @@ private CloseableIterable<FileScanTask> 
planTableScan(PlanTableScanRequest planT
 
   private FileIO scanFileIO(List<Credential> storageCredentials) {
     ImmutableMap.Builder<String, String> builder =
-        ImmutableMap.<String, String>builder().putAll(catalogProperties);
+        ImmutableMap.<String, String>builder()
+            .putAll(catalogProperties)
+            .put(
+                RESTCatalogProperties.REMOTE_SIGNING_ENDPOINT,
+                resourcePaths.remoteSign(tableIdentifier));
+
+    // If the original FileIO has a remote signing config property,
+    // we need to propagate it to the scan FileIO
+
+    @SuppressWarnings("resource")
+    Map<String, String> ioProperties = operations.io().properties();
+
+    if (ioProperties.containsKey(RESTCatalogProperties.REMOTE_SIGNING_CONFIG)) 
{

Review Comment:
   We should only set this if storageCredentials are not set.  If the scan 
contains credentials, that's an explicit signal from the catalog to use those 
over remote signing.
   
   I don't know what order these would be applied in the different FileIO 
impls, so we should be clear the explicit credentials override.



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