1996fanrui commented on code in PR #929:
URL:
https://github.com/apache/flink-kubernetes-operator/pull/929#discussion_r1924588749
##########
flink-autoscaler-plugin-jdbc/src/test/java/org/apache/flink/autoscaler/jdbc/testutils/databases/derby/DerbyExtension.java:
##########
@@ -76,7 +79,8 @@ public void beforeAll(ExtensionContext extensionContext)
throws Exception {
var jobKeyReasonCreateTimeIndex =
"CREATE INDEX job_key_reason_create_time_idx ON
t_flink_autoscaler_event_handler (job_key, reason, create_time)";
- try (var conn = getConnection();
+ var dataSource = getDataSource();
+ try (var conn = dataSource.getConnection();
Review Comment:
```suggestion
try (var dataSource = getDataSource();
var conn = dataSource.getConnection();
```
##########
flink-autoscaler-standalone/src/test/java/org/apache/flink/autoscaler/standalone/AutoscalerEventHandlerFactoryTest.java:
##########
@@ -68,14 +68,14 @@ void testCreateJdbcEventHandler() throws Exception {
final var conf = new Configuration();
conf.set(EVENT_HANDLER_TYPE, JDBC);
conf.set(JDBC_URL, String.format("%s;create=true", jdbcUrl));
- HikariJDBCUtil.getConnection(conf).close();
+ HikariJDBCUtil.getDataSource(conf).close();
var eventHandler = AutoscalerEventHandlerFactory.create(conf);
assertThat(eventHandler).isInstanceOf(JdbcAutoScalerEventHandler.class);
+ conf.set(JDBC_URL, String.format("%s;shutdown=true", jdbcUrl));
- try {
- conf.set(JDBC_URL, String.format("%s;shutdown=true", jdbcUrl));
- HikariJDBCUtil.getConnection(conf).close();
+ try (var datasource = HikariJDBCUtil.getDataSource(conf)) {
Review Comment:
It's similar with the last comment.
##########
flink-autoscaler-standalone/src/test/java/org/apache/flink/autoscaler/standalone/AutoscalerStateStoreFactoryTest.java:
##########
@@ -67,14 +66,12 @@ void testCreateJdbcStateStore() throws Exception {
final var conf = new Configuration();
conf.set(STATE_STORE_TYPE, JDBC);
conf.set(JDBC_URL, String.format("%s;create=true", jdbcUrl));
- HikariJDBCUtil.getConnection(conf).close();
var stateStore = AutoscalerStateStoreFactory.create(conf);
assertThat(stateStore).isInstanceOf(JdbcAutoScalerStateStore.class);
try {
conf.set(JDBC_URL, String.format("%s;shutdown=true", jdbcUrl));
- HikariJDBCUtil.getConnection(conf).close();
Review Comment:
Both close connection and datasource should be wrapped inside of try to
prevent test failure.
We can check the comment: `database shutdown ignored exception`, this catch
means the test won't be failed if database shutdown throws exception.
--
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: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
