Don't try and grab a connection after it (may) is established.

[jonathan]

This is so that you can override the datasource (e.g. mock) and not get an error.

[maxmmurphy]

Waiting an hour for any objections. Otherwise looks good to me.

[jballanc]

Question: is calling establish_connection on ActiveRecord::Base really the right thing to do anyway? I believe the connection property of ActiveRecord::Base is a class_attribute, which means that any new settings will be inherited by any subclasses that have not already specified their own setting (check out the source in ActiveSupport).

So, for example, you might have:

class UserModel < Peacekeeper::Model; end
class RestaurantModel < Peacekeeper::Model; end

Peacekeeper::Model.config = generic_config
Peacekeeper::Model.data_source = :active_record
# ^^^ At this point, both UserModel and RestaurantModel are connected to the DB specified in generic_config
RestaurantModel.config = restaurant_db_config
RestaurantModel.data_source = :active_record
# ^^^ The intention was to load Restaurants from a specific DB, but now both User and Restaurant are pointed at restaurant_db_config

If you read the docs on ActiveRecord::Base.establish_connection, you should find that subclasses can establish a connection on their own. So you probably want something more like @data_class.establish_connection ... but don't take my word for it ;)

[vosechu]

If you want models to be able to connect to other things they can be set on their own with @data_class.establish_connection. If that's not something you care about then it seems like setting it on AR:B would be the correct way.

[jonathan]

Removing the connection code doesn't fix the problem. Trying to get the connection right after establishing it just allowed the problem to come to a head more quickly.

@jballanc, we can check for a data_class first and, if it isn't nil, then establish the connection. If it isn't nil, we can set it on the data_class. But I don't think that fixes the root of the problem.

The root of the problem, as I see it, is that setting the the data_source tries to make a connection to the DB and it's assuming that connection is lazy. It won't actually try to connect until there is a DB query. AR is greedy and tries to establish a connection immediately. I don't know a ton about sequel so correct me if I'm wrong, @jballanc.

It might be best to clearly state whether or not data_source should be lazy or greedy. I'd vote for lazy, but that means more work getting AR working.

[jballanc]

Ok, so obviously a little more digging into AR is in order.

I agree that data connection should be as lazy as possible. It is not completely inconceivable that in a dev environment or testing (as in the case where you discovered this issue), the data_source value may change a handful of times before an actual "connection" is made.

I would not be surprised if AR is making a query as part of establish_connection. AR does a lot of weird stuff...if this is the case, we will probably have to figure out how to special-case AR. Probably we could create a proxy connection object that holds all the info for creating the connection, but doesn't actually call through to establish_connection until the connection is actually needed. At that point, it could connect and get out of the way.

Another possibility is that we could look at the guts of establish_connection and replicate them lazily in PK...as I said, more investigation needed :-/