laserninja commented on code in PR #11634:
URL: https://github.com/apache/gravitino/pull/11634#discussion_r3920607402
##########
iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/FederatedCatalogWrapper.java:
##########
@@ -188,12 +198,109 @@ public PlanTableScanResponse planTableScan(
catalogCredentialManager.catalogName(), tableIdentifier, response);
}
+ /**
+ * Reports whether the remote catalog advertises the scan-plan endpoint,
since {@link
+ * #planTableScan} delegates planning to it rather than planning locally.
+ *
+ * <p>The answer comes from the remote catalog's own {@code /v1/config}
response and is cached for
+ * the lifetime of this wrapper, so the remote is queried at most once
rather than on every local
+ * {@code /v1/config} call.
+ *
+ * <p>A remote that omits {@code endpoints} is treated as not supporting
scan planning. That
+ * matches the Iceberg client, which falls back to a default endpoint set
that predates scan
+ * planning when the field is absent.
+ *
+ * <p>If the remote cannot be reached the result is not cached and the
endpoint is not advertised,
+ * so a later call can still resolve it once the remote recovers. Not
advertising is the safe
+ * direction here: the endpoint would fail anyway while the remote is
unreachable.
+ *
+ * @return {@code true} if the remote catalog advertises {@code
V1_SUBMIT_TABLE_SCAN_PLAN}.
+ */
+ @Override
+ public boolean supportsScanPlanOperations() {
+ Boolean cached = remoteSupportsScanPlan;
+ if (cached != null) {
+ return cached;
+ }
+
+ try {
+ boolean supported =
+
fetchRemoteConfig().endpoints().contains(Endpoint.V1_SUBMIT_TABLE_SCAN_PLAN);
+ remoteSupportsScanPlan = supported;
+ return supported;
+ } catch (Exception e) {
+ LOG.warn(
+ "Failed to read the endpoints advertised by the remote catalog of
{}; not advertising the"
+ + " scan plan endpoint",
+ catalogCredentialManager.catalogName(),
+ e);
+ return false;
+ }
+ }
+
+ /**
+ * Fetches the remote catalog's {@code /v1/config} response.
+ *
+ * <p>The {@code warehouse} query parameter is forwarded when configured, so
a remote serving
+ * several warehouses returns the endpoint set for the one this catalog
federates.
+ *
+ * @return the remote catalog's config response.
+ */
+ @VisibleForTesting
+ ConfigResponse fetchRemoteConfig() {
Review Comment:
Both done in 57ccb8ab6.
1. `fetchRemoteConfig` is now `private`. The `@VisibleForTesting` import
stays since it is still used elsewhere in the file.
2. Added the javadoc paragraph you suggested, near-verbatim.
--
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]
