Added custom configuration to slave connections #113
Conversation
Do you need that, can't you just send a lambda that creates an object like the one you need? |
|
It's mostly for tests, as I want to assert that the mysql connection is created with the correct config. |
Is kinda OK in this context to reach into the object via |
|
ping @arthurnn not sure if you are in context, do ask if any of this makes no sense. |
| config = ActiveRecord::Base.connection_pool.spec.config.dup | ||
| config[:host] = slave | ||
| ActiveRecord::Base.send(adapter_method, config) | ||
| if @slave_connection.nil? |
There was a problem hiding this comment.
unless @slave_connection should be enough .
|
I've updated according to feedback and added unittests. However, I'm unsure what how this code triggered failed test? Also I can't get test to run locally according to the (outdated) instructions under spec/README. Any feedback? |
| end | ||
|
|
||
| it 'should use the custom slave connection' do | ||
| assert_equal(@connection, @throttler.send(:slave_connection, "slave.db.local")) |
There was a problem hiding this comment.
assert_ and friends are normally used sans parentheses
There was a problem hiding this comment.
how does this assertion warranty that the connection is custom?
This commit allows for custom connection to be passed when establishing connection to slaves. While we can assume that with traditional rails apps, a straightforward host substitution will be suffice, this assumption cannot be made for projects of a certain scale, where slave boxes may have custom configurations (maybe different password/username/etc). This commit's approach relies on the consumer of this package to specify a lambda that takes in the slave hostname and returns a connection object.
This commit allows for custom configurations to be passed when
establishing connection to slaves.
While we can assume that with traditional rails apps, a straightforward
host substitution will be suffice, this assumption cannot be made for
projects of a certain scale, where slave boxes may have custom
configurations (maybe different password/username/etc).
This commit's approach relies on the consumer of this package to specify
a lambda that takes in the slave hostname and returns a configuration
object to pass to Mysql2Adapter. Alternatively, we could make the
consumer pass in a hash, but I think this approach is slightly cleaner
and more flexible.
@camilo @arthurnn @jasonhl
Also note: I don't quite know how to properly test this. It seems like the MysqlAdapter does not expose its config object in something that I can access..