lasdf1234 commented on code in PR #10800:
URL: https://github.com/apache/gravitino/pull/10800#discussion_r3526848887
##########
iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/FederatedCatalogWrapper.java:
##########
@@ -153,6 +155,49 @@ public LoadCredentialsResponse getTableCredentials(
catalogCredentialManager.catalogName(), identifier, upstream);
}
+ /**
+ * Plans a table scan and, when credential vending is requested, fetches
credentials from the
+ * remote REST catalog rather than using the local credential manager.
+ *
+ * @param tableIdentifier the table to scan.
+ * @param scanRequest the scan request parameters.
+ * @param requestCredentialVending whether the client requested vended
credentials.
+ * @param privilege ignored; the remote REST catalog decides what to vend.
+ * @return the scan response, optionally enriched with remote credentials.
+ */
+ @SuppressWarnings("deprecation")
+ @Override
+ public PlanTableScanResponse planTableScan(
+ TableIdentifier tableIdentifier,
+ PlanTableScanRequest scanRequest,
+ boolean requestCredentialVending,
+ CredentialPrivilege privilege) {
+ PlanTableScanResponse response =
+ super.planTableScan(tableIdentifier, scanRequest, false, privilege);
+ if (!requestCredentialVending) {
+ return response;
+ }
+ try {
+ LoadCredentialsResponse upstream =
+ getRESTTableCredentials((RESTCatalog) getCatalog(), tableIdentifier);
+ LoadCredentialsResponse rewritten =
+ IcebergRESTUtils.rewriteTableCredentials(
+ catalogCredentialManager.catalogName(), tableIdentifier,
upstream);
+ return PlanTableScanResponse.builder()
+ .withPlanStatus(response.planStatus())
+ .withPlanId(response.planId())
+ .withPlanTasks(response.planTasks())
+ .withFileScanTasks(response.fileScanTasks())
+ .withSpecsById(response.specsById())
+ .withErrorResponse(response.errorResponse())
+ .withCredentials(rewritten.credentials())
+ .build();
+ } catch (Exception e) {
+ LOG.warn("Failed to fetch remote credentials for scan of table: {}",
tableIdentifier, e);
+ return response;
+ }
+ }
+
Review Comment:
No need for try catch,Could you change the code to the following format?
@SuppressWarnings("deprecation")
@Override
public PlanTableScanResponse planTableScan(
TableIdentifier tableIdentifier,
PlanTableScanRequest scanRequest,
boolean requestCredentialVending,
CredentialPrivilege privilege) {
PlanTableScanResponse response =
super.planTableScan(tableIdentifier, scanRequest, false, privilege);
if (!requestCredentialVending) {
return response;
}
LoadCredentialsResponse credentials = getTableCredentials(tableIdentifier,
privilege);
if (credentials.credentials().isEmpty()) {
return response;
}
return PlanTableScanResponse.builder()
.withPlanStatus(response.planStatus())
.withPlanId(response.planId())
.withPlanTasks(response.planTasks())
.withFileScanTasks(response.fileScanTasks())
.withSpecsById(response.specsById())
.withErrorResponse(response.errorResponse())
.withCredentials(credentials.credentials())
.build();
}
--
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]
