phet commented on code in PR #3567:
URL: https://github.com/apache/gobblin/pull/3567#discussion_r977340366
##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/spec_store/MysqlBaseSpecStore.java:
##########
@@ -325,6 +327,7 @@ protected <T> T withPreparedStatement(String sql,
CheckedFunction<PreparedStatem
}
return result;
} catch (SQLException e) {
+ log.warn("Received SQL exception that can result from invalid
connection. Checking if validation query is set {} Exception is {}",
((BasicDataSource) this.dataSource).getValidationQuery());
Review Comment:
good thinking!
##########
gobblin-utility/src/main/java/org/apache/gobblin/util/jdbc/DataSourceProvider.java:
##########
@@ -37,6 +38,7 @@
/**
* A provider class for {@link javax.sql.DataSource}s.
*/
+@Slf4j
Review Comment:
curious, do we need this in addition to the `LoggerFactory.getLogger` call?
##########
gobblin-utility/src/main/java/org/apache/gobblin/util/jdbc/DataSourceProvider.java:
##########
@@ -81,6 +83,7 @@ public DataSourceProvider(@Named("dataSourceProperties")
Properties properties)
}
public DataSourceProvider() {
+ log.warn("Creating {} without setting validation query",
this.getClass().getSimpleName());
Review Comment:
another great idea. might consider including
`Thread.currentThread().getStackTrace()` to better pinpoint context.
##########
gobblin-metastore/src/main/java/org/apache/gobblin/metastore/JobHistoryDataSourceProvider.java:
##########
@@ -33,6 +33,7 @@ public class JobHistoryDataSourceProvider extends
org.apache.gobblin.util.jdbc.D
@Inject
public JobHistoryDataSourceProvider(@Named("dataSourceProperties")
Properties properties) {
+ super(properties);
Review Comment:
should we worry that perhaps some settings within the base class ctor (like
`.setUsername` or `.setPassword`) wouldn't be overridden here, if the config
keys used by the base class are present, but the ones used by this derived
class are not?
--
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]
