yuqi1129 commented on code in PR #10081:
URL: https://github.com/apache/gravitino/pull/10081#discussion_r2966494968
##########
catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcCatalog.java:
##########
@@ -113,4 +118,26 @@ public PropertiesMetadata schemaPropertiesMetadata()
throws UnsupportedOperation
public PropertiesMetadata tablePropertiesMetadata() throws
UnsupportedOperationException {
return TABLE_PROPERTIES_META;
}
+
+ @Override
+ public Map<String, String> propertiesWithCredentialProviders() {
+ Map<String, String> properties =
Maps.newHashMap(super.propertiesWithCredentialProviders());
+ return buildCredentialProvidersIfNecessary(properties);
+ }
+
+ private Map<String, String> buildCredentialProvidersIfNecessary(Map<String,
String> properties) {
+ // If credential providers already set, return as is
+ if
(StringUtils.isNotBlank(properties.get(CredentialConstants.CREDENTIAL_PROVIDERS)))
{
+ return properties;
+ }
+
+ // Add JDBC credential provider if jdbc-user and jdbc-password are set
+ String jdbcUser = properties.get(JdbcConfig.USERNAME.getKey());
+ String jdbcPassword = properties.get(JdbcConfig.PASSWORD.getKey());
+ if (StringUtils.isNotBlank(jdbcUser) &&
StringUtils.isNotBlank(jdbcPassword)) {
+ properties.put(CredentialConstants.CREDENTIAL_PROVIDERS,
JdbcCredential.JDBC_CREDENTIAL_TYPE);
+ }
Review Comment:
**Architectural inconsistency with `IcebergCatalog` and `PaimonCatalog`**:
both of those catalogs build a _list_ of credential providers (JDBC +
S3/OSS/Azure) and join them with commas. `JdbcCatalog` only ever sets a single
`JDBC_CREDENTIAL_TYPE`.
This is fine for a pure JDBC catalog, but the inconsistency means if someone
extends `JdbcCatalog` and the underlying storage also needs S3 credentials,
there is no hook to add them. Consider aligning the implementation structure
with `IcebergCatalog.buildCredentialProvidersIfNecessary()` — even if the
result is the same today, it is easier to extend later.
--
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]
