New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add settings for removeAbandonedTimeout and removeAbandoned #1264

Merged
merged 1 commit into from Sep 28, 2015

Conversation

Projects
None yet
2 participants
@joschi
Member

joschi commented Sep 28, 2015

Fixes #1255

@joschi joschi added the improvement label Sep 28, 2015

@joschi joschi added this to the 0.9.0 milestone Sep 28, 2015

@@ -769,6 +813,12 @@ public ManagedDataSource build(MetricRegistry metricRegistry, String name) {
poolConfig.setUrl(url);
poolConfig.setUsername(user);
poolConfig.setPassword(user != null && password == null ? "" : password);
poolConfig.setRemoveAbandoned(removeAbandoned);
if (getRemoveAbandonedTimeout().isPresent()) {

This comment has been minimized.

@arteam

arteam Sep 28, 2015

Member

Maybe we could set the timeout unconditionally? We already set the default value the same as in the pool configuration, if the user doesn't specify the property.

@arteam

This comment has been minimized.

Member

arteam commented Sep 28, 2015

Could you update DataSourceConfigurationTest, so we can verify that deserialization works correctly and the default values are applied? Apart from that, the PR looks good to me.

@joschi joschi force-pushed the issue-1255 branch from 2964adc to d6d84c4 Sep 28, 2015

arteam added a commit that referenced this pull request Sep 28, 2015

Merge pull request #1264 from dropwizard/issue-1255
Add settings for removeAbandonedTimeout and removeAbandoned

@arteam arteam merged commit 4e91753 into master Sep 28, 2015

@arteam

This comment has been minimized.

Member

arteam commented Sep 28, 2015

Thanks!

@joschi joschi deleted the issue-1255 branch Sep 28, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment