Skip to content
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

(feat) support max connection setting for database configuration #7520

Closed
wants to merge 1 commit into from

Conversation

huydx
Copy link
Contributor

@huydx huydx commented Feb 10, 2017

Fix #7427

Copy link
Contributor

@bergquist bergquist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! But this code could be simplify a bit.

@@ -177,6 +198,18 @@ func LoadConfig() {
DbCfg.Host = sec.Key("host").String()
DbCfg.Name = sec.Key("name").String()
DbCfg.User = sec.Key("user").String()
DbCfg.MaxConn, err = sec.Key("max_conn").Int()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use .MustInt(0) instead. Reduce the amount of code needed to read the setting. 0 is the default value anyway :)

if err != nil {
return nil, err
} else {
if DbCfg.MaxConn != 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if statement is kinda of redundant. xorm will ignore any connection setting that is 0. So just pass in the DbCfg.MaxConn value. Same for MaxOpenConn and MaxIdleConn

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, didn't know that, thanks for the information!

@huydx
Copy link
Contributor Author

huydx commented Feb 10, 2017

fixed

@bergquist
Copy link
Contributor

bergquist commented Feb 10, 2017

Merged! Thank you for contributing! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/changes-needed pr/external This PR is from external contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature request] Allow max db connection pool configuration
3 participants