Skip to content

Conversation

@lcx
Copy link

@lcx lcx commented May 6, 2015

Added configuration option for pool_timeout and max_connections to pass over to sequel

Copy link
Owner

Choose a reason for hiding this comment

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

Tiny nit, but would you please add spaces after the commas?

Copy link
Owner

Choose a reason for hiding this comment

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

Also, it occurs to me that it might be better to pass this as a hash to #establish_connection, in case we get more params in the future. The calling code would then look something like

@@connection = establish_connection connection_string(params), max_connections: max_connections, pool_timeout: pool_timeout

@bklang
Copy link
Owner

bklang commented May 6, 2015

A few code nits, but thank you for your contribution!

@lcx
Copy link
Author

lcx commented May 7, 2015

Odd, no idea why I used ruby 1.8 syntax, I occasionally do that :)
Should I fix the issues or will you fix it?
Survived in production today without PoolTimeout exception with a bunch of calls to a radio station. So looks stable I would say.

@vangberg
Copy link

vangberg commented May 7, 2015

There are a lot of adapter specific connection options[1], so I'm wondering if it wouldn't make sense to take a connection_options configuration key, and pass verbatim to Sequel.connect?

[1] http://sequel.jeremyevans.net/rdoc/files/doc/opening_databases_rdoc.html

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants