yuqi1129 commented on code in PR #14485:
URL: https://github.com/apache/gravitino/pull/14485#discussion_r4441080792
##########
catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/utils/DataSourceUtils.java:
##########
@@ -88,51 +89,68 @@ public static DataSource createDataSource(JdbcConfig
jdbcConfig)
// config to the factory. Kept outside the try below so the specific
reason surfaces directly
// instead of being wrapped as "Error creating datasource".
rejectUnsafePoolProperties(jdbcConfig.getAllConfig());
+ // Validate user configuration before the data-source builder wraps
infrastructure failures.
+ jdbcConfig.getVerifyOnCreate();
+ jdbcConfig.getTestOnBorrow();
+ jdbcConfig.getConnectTimeoutMs();
+ jdbcConfig.getMaxWaitMs();
Review Comment:
Addressed in `9fa58b2df7`, with follow-ups in `fbee69d94b` and `f3a483b5df`
(all included in the current head, `7b5c790126`).
- Create now runs the configuration-only validation hook independently of
`verify-on-create`, so disabling the connection probe does not bypass
validation.
- Alter validates the complete proposed configuration before writing secrets
or persisting the update. Rejection preserves the previous properties and
secrets.
- JDBC catalogs use the shared `DataSourceUtils.validateConfiguration` path.
Unsafe URL/driver/pool checks still precede configured-driver instantiation;
static validation does not open a source connection.
Regression coverage includes
`testConfigurationValidationOnCreateWithoutBackendVerification`,
`testConfigurationValidationOnAlterUsesCompleteProposedProperties`, and
`testInvalidConfigurationAlterDoesNotMutateSecrets`. The first two deliberately
disable backend initialization, so they exercise static validation
independently of the connection probe.
The original create/alter validation gap is fixed.
--
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]
