dimas-b commented on code in PR #4052:
URL: https://github.com/apache/polaris/pull/4052#discussion_r2984736272
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -812,6 +816,13 @@ public Optional<LoadTableResponse> loadTable(
// when data-access is specified but access delegation grants are not
found.
Table table = baseCatalog.loadTable(tableIdentifier);
+ // Capture tableId from remote catalog response for S3 Tables ARN
construction
+ Optional<String> capturedTableId = capturedConfigHolder().getTableId();
+ List<String> resourceArns = List.of();
+ if (capturedTableId.isPresent()) {
+ resourceArns = List.of(constructS3TablesArn(capturedTableId.get()));
Review Comment:
This is a smart idea 😉 However, I'm not sure it is viable from the internal
APIs POV. This approach appears to circumvent the usual way of passing
locations to the storage integration code and instead introduces a new optional
way of passing similar information via a different channel
(`CredentialVendingContext`).
I believe it would be beneficial to holistically refactor all storage
integrations call paths so that location data is handled uniformly.
Note: such a refactoring will probably overlap with #3699.
--
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]
