adutra commented on code in PR #3480:
URL: https://github.com/apache/polaris/pull/3480#discussion_r2753458957
##########
polaris-core/src/main/java/org/apache/polaris/core/catalog/ExternalCatalogFactory.java:
##########
@@ -33,24 +35,93 @@ public interface ExternalCatalogFactory {
/**
* Creates a catalog handle for the given connection configuration.
*
+ * @deprecated Implement {@link #createCatalog(ConnectionConfigInfoDpo,
PolarisCredentialManager,
+ * Map)} instead to support catalog properties pass-through (e.g., proxy
settings).
* @param connectionConfig the connection configuration
* @param polarisCredentialManager the credential manager for generating
connection credentials
* that Polaris uses to access external systems
* @return the initialized catalog
* @throws IllegalStateException if the connection configuration is invalid
*/
+ @Deprecated
Catalog createCatalog(
ConnectionConfigInfoDpo connectionConfig, PolarisCredentialManager
polarisCredentialManager);
+ /**
+ * Creates a catalog handle for the given connection configuration.
+ *
+ * <p>Implementations should override this method to receive catalog
properties (e.g.,
+ * rest.client.proxy.*, timeout settings). The default implementation
delegates to the deprecated
+ * 2-parameter version and logs a warning if properties are being ignored.
+ *
+ * @param connectionConfig the connection configuration
+ * @param polarisCredentialManager the credential manager for generating
connection credentials
+ * that Polaris uses to access external systems
+ * @param catalogProperties additional properties from the ExternalCatalog
entity that should be
+ * passed through to the underlying catalog (e.g., rest.client.proxy.*,
timeout settings).
+ * These are merged with lower precedence than connection config
properties.
+ * @return the initialized catalog
+ * @throws IllegalStateException if the connection configuration is invalid
+ */
+ default Catalog createCatalog(
+ ConnectionConfigInfoDpo connectionConfig,
+ PolarisCredentialManager polarisCredentialManager,
+ Map<String, String> catalogProperties) {
+ if (catalogProperties != null && !catalogProperties.isEmpty()) {
+ LoggerFactory.getLogger(getClass())
+ .warn(
+ "catalogProperties were provided but {} does not override
createCatalog with "
Review Comment:
I am OK with the breaking change approach, FYI.
--
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]
