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

Configurable options to Sequel.connect #123

Conversation

olleolleolle
Copy link
Member

@olleolleolle olleolleolle commented Nov 14, 2016

This PR adds a configuration key db_connection_options, which is expected to be a Hash of Sequel.connect options.

See #115

@olleolleolle olleolleolle force-pushed the feature/sequel-config-as-db_connection_options branch from ef3751a to 2951526 Compare November 14, 2016 22:22
Valid values
------------

A valid connection options Hash for the [Sequel.connect](http://sequel.jeremyevans.net/rdoc/files/doc/opening_databases_rdoc.html#label-General+connection+options) method.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please move the changes to files in the docs directory into the corresponding files in the man directory? Those end up generating these 😄

end
when "postgres", "mysql", "mysql2"
db = Sequel.connect(config[:db_url])
db = Sequel.connect(config[:db_url], config[:db_connection_options])
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about defaulting to the number of puma threads + 1 as @chriseckhardt ended up using? Perhaps { max_connections: config[:puma_threads] + 1 }.merge(config[:db_connection_options])? This would mean adding a new :puma_threads config variable as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Splendid idea, will take it up immediately.

@olleolleolle olleolleolle force-pushed the feature/sequel-config-as-db_connection_options branch from af424f5 to 0072dd8 Compare November 17, 2016 21:59
@olleolleolle
Copy link
Member Author

@smellsblue Does it look right to you, now?

@smellsblue smellsblue merged commit f48137f into rubygems:master Dec 8, 2016
@smellsblue
Copy link
Contributor

@olleolleolle perfect! thanks so much!

@olleolleolle olleolleolle deleted the feature/sequel-config-as-db_connection_options branch December 8, 2016 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants