schema_plus 2.0.0.pre2 in Padrino throws NoMethodError when encountering FKs

[fj]

When running db:schema:load and encountering a foreign key, the prerelease version throws a NoMethodError:

-- create_table("widgets", {:id=>false, :force=>true})
rake aborted!
NoMethodError: undefined method `foreign_key' for #<ActiveRecord::ConnectionAdapters::PostgreSQL::TableDefinition:0x007f7c720e67d0>

You can reproduce this by just having

create_table "...", force: true do |t|
  t.foreign_key ...
end

in a schema and running db:schema:load.

[ronen]

@fj First, thanks for trying out the prerelease!

But hmm, i'm not able to reproduce that.

It's vaguely possible it was fixed by a fix i pushed to schema_monkey earlier today. If you're not using schema_monkey 0.3.2 could you bundle update and try again?

If that doesn't fix it, can you send me more details? ruby version, Gemfile.lock, schema.rb. or even the entire failing app if that's something you can distribute. (email or gist is fine)

[fj]

Sure. I'll investigate this today and see if I can get something for you. I confirmed the bug is still there (for me) even after updating.

[ronen]

Thanks.

FYI I just released 2.0.0.pre2, can you try it with that instead? I don't expect it to fix your problem. But then again, I don't know what's causing your problem, so it's hard to say :)

BTW 2.0.0.pre1 won't work any more if you update, because of a breaking change in schema_monkey 0.3.0 -> 0.4.0 (I should have made a major version bump in schema_monkey, but since the only thing that I know of that it breaks is the prerelease, I let it slide.)

[fj]

I updated to schema_plus 2.0.0.pre2. With the following schema, I get the error every time using ActiveRecord 4.2, and running rake ar:drop ar:create ar:schema:load in Padrino:

ActiveRecord::Schema.define(version: 20141229170500) do

  # These are extensions that must be enabled in order to support this database
  enable_extension "plpgsql"

  create_table "anomalies", force: true do |t|
    t.datetime "created_at", null: false
  end

  create_table "predictions", force: true do |t|
    t.integer  "observation_id",                             null: false
    t.string   "observation_type",                           null: false
    t.decimal  "value",            precision: 30, scale: 10, null: false
    t.decimal  "high",             precision: 30, scale: 10, null: false
    t.decimal  "low",              precision: 30, scale: 10, null: false
    t.datetime "created_at",                                 null: false
    t.index ["created_at"], :name => "index_predictions_on_created_at"
    t.index ["observation_id", "observation_type"], :name => "index_predictions_on_observation_id_and_observation_type", :unique => true
  end

  create_table "anomalies_predictions", id: false, force: true do |t|
    t.integer "anomaly_id",    null: false
    t.integer "prediction_id", null: false
    t.index ["anomaly_id"], :name => "fk__anomalies_predictions_anomaly_id"
    t.index ["prediction_id"], :name => "fk__anomalies_predictions_prediction_id"
    t.index ["prediction_id"], :name => "index_anomalies_predictions_on_prediction_id", :unique => true
    t.foreign_key ["anomaly_id"], "anomalies", ["id"], :on_update => :no_action, :on_delete => :no_action, :name => "fk_anomalies_predictions_anomaly_id"
    t.foreign_key ["prediction_id"], "predictions", ["id"], :on_update => :no_action, :on_delete => :no_action, :name => "fk_anomalies_predictions_prediction_id"
  end
end

and the error is:

-- create_table("anomalies_predictions", {:id=>false, :force=>true})
rake aborted!
NoMethodError: undefined method `foreign_key' for #<ActiveRecord::ConnectionAdapters::PostgreSQL::TableDefinition:0x007f0f10509b20>

[fj]

Does schema_plus add methods to AR's ConnectionAdapters? I wonder if they're not being included somehow.

[ronen]

Yes it does -- actually in this case the missing methods are supposed to be added to TableDefinition rather than ConnectionAdapters.

Which db gem / connection adapter are you using? vanilla "pg" or are you using postgis or some other?

[fj]

Just vanilla pg. Also, sorry; yes, I meant TableDefinition (that's what's reflected in the error, too).

[ronen]

Oh, I just noticed: Padrino. That explains it. SchemaPlus (actually SchemaMonkey) relies on a rails Railtie to insert itself. Without rails, SchemaPlus isn't being included.

I'm not terribly familiar with Padrino, so I'm not certain the appropriate way to do this, but you need to arrange to call SchemaMonkey.insert sometime after ActiveRecord is loaded but before the database connection is created.

For rails, that actually happens in two places: in an on_load(:active_record) callback in the rails initialization process, and also as an extra schema_monkey:insert task that gets injected into the db:schema:load & db:schema:drop tasks. It all happens here: https://github.com/SchemaPlus/schema_monkey/blob/master/lib/schema_monkey/railtie.rb

Do you want to try to get it working for Padrino? If there's some elegant way to package it up for Padrino I'd be happy to include it in SchemaPlus/SchemaMonkey.

Now that you bring this up, SchemaPlus has no reason for an explicit dependency on rails, it should just depend on activerecord. But it should recognize if rails is loaded and if so do the Railtie thing. And ideally similarly recognize if Padrino is loaded and if so do whatever the Padrino thing is

[fj]

Yeah, Padrino doesn't really have anything like Railties unless you're integrating with something Padrino-specific (which you aren't). So I don't think this would be too hard to make work -- it looks like the only thing that actually needs to happen is a call to SchemaMonkey.insert plus enhancing the Rake tasks.

Generally the convention I see is that if people want to support different framework-specific integrations, one does something like require 'schema_plus/rails' to get Rails-specific things, require 'schema_plus/padrino' to get Padrino specific things, and so on. Otherwise your gem has to "detect" the usage of Rails/Padrino/... in some way, which is more prone to breakage than just having the user say what they want.

There's also the gem-specific route (e.g. rspec also has rspec-rails), but I find that to be annoying as a maintainer.

Anyway, if you're okay with unconditional loads then the Padrino integration is probably just this one-liner:

# padrino.rb
SchemaMonkey.insert

# your app
require 'schema_plus/integrations/padrino' # or whatever

and then the Rake tasks are something like

# padrino/rake_tasks.rb

      namespace :schema_monkey do
        task :insert do
          SchemaMonkey.insert
        end
      end
      ['db:schema:dump', 'db:schema:load'].each do |name|
        if task = Rake.application.tasks.find { |task| task.name == name }
          task.enhance(["schema_monkey:insert"])
        end
      end

# your app's Rakefile
require 'schema_plus/integrations/padrino/rake_tasks' # or whatever

WDYT?

[fj]

(Of course, that all presumes that SchemaMonkey.insert isn't doing something Rails-specific, but I didn't dig deeply into that.)

[ronen]

Your point about detecting Rails/Padrino/etc is well taken. But checking defined?(Rails::Railtie) is reasonably venerable too. And since I'd like schema_plus 2.0 to be drop-in compatible with 1.8 (albeit with deprecations) I'd like to keep rails auto-insertion in there (at least for now, maybe deprecate it at some point).

But yes, for Padrino it may as well be explicit. Including the integrations in schema_plus isn't great since schema_plus is transitioning to just pulling in the collection of schema_plus_xxxx gems, but not having any code of its own.

So how about a new gem schema_plus_padrino that you would load in addition to schema_plus (or whichever collection of schema_plus_xxxx gems you use)? I could imagine it would be something like this:

# schema_plus_padrino.rb
module SchemaPlusPadrino
  def self.insert
    SchemaMonkey.insert
  end
end

# schema_plus_padrino/rake.rb
require 'schema_monkey/rake'
SchemaMonkey::Rake.insert("ar:schema:dump", "ar:schema:load")

and integration would be like:

# your Gemfile
gem "schema_plus"
gem "schema_plus_padrino"

# your app
SchemaPlusPadrino.insert

# your Rakefile
require 'schema_plus_padrino/rake'

WDYT?

[fj]

Sure, that'd work. That's what I was describing with the rspec/rspec-rails analogy before, so it definitely makes sense to me -- but like I said, I think it's slightly more work for you (later, you'll be maintaining a schema_plus_rails, a schema_plus_padrino, etc.).

The only small correction is that you'd probably want it to be SchemaPlus::Padrino, not SchemaPlusPadrino.

[ronen]

I think it's slightly more work for you

thanks for the concern! honestly, i'm hoping/planning that by breaking schema_plus into a bunch of small pieces, it'll be overall easier to maintain. and hopefully i won't have to be the one doing the heavy lifting of maintaining them all; if nothing else having small single-purpose gems should make it easier for people to submit PRs

(BTW If you're planning on using schema_plus 2.x with padrino, would you like to be the owner of schema_plus_padrino?)

The only small correction is that you'd probably want it to be SchemaPlus::Padrino, not SchemaPlusPadrino.

Yeah, I've been going back and forth on that. Since I'm making a whole family of gems named SchemaPlusXxxx, should they all be in a SchemaPlus namespace? That has a certain aesthetic appeal. On the other hand, the gems are all supposed to be independent of each other, and I do really like the strict convention that a gem defines its own top-level module.

[fj]

I'd be happy to share responsibility, but I think you should at least be co-maintainer (otherwise, if you needed to fix something and you couldn't reach me on short notice, that would be bad).

[ronen]

FYI I made schema_monkey_rails, pulling the Railtie stuff out of [schema_monkey](https://github.com/SchemaPlus/schema_monkey.

What pushed me over the edge was realizing that making a proper rspec for the Railtie requires pulling in rails, which muddies up the test environment; by pulling the Railtie out, schema_monkey now tests against just ActiveRecord, ensuring there's no hidden Rails dependency.

(My backwards-compatibility thing still works because the schema_plus gem will now require schema_monkey_rails. But anybody who uses one or more of the schema_plus_xxxx gems without the schema_plus wrapper will need to require schema_monkey_rails if they're using rails.)

So at some point you or I can make schema_monkey_padrino, which will define SchemaMonkey::Padrino.insert etc.

[fj]

Thanks @ronen. I'm going to start with just a couple of lib files in my app that mirror that would be in schema_plus_padrino anyway. If that works well I'll ping you again here and ask you to make the repo (I think it should be under the SchemaPlus org, rather than my personal repo, if you don't mind).

[fj]

Just FYI, the integration I proposed above seems to work in our application. This is everything; it's pretty short:

[ronen]

@fj I've created schema_monkey_padrino, along the above lines, and listed you as a co-owner (gem) and collaborator (github).

Since I don't really know Padrino, could you....

• ...take a look at the README and make sure I got it right?
• ...test it out in your app and make sure I got it right?
• ...flesh out the specs? I left in some boilerplate based on schema_monkey_rails in case that's a useful starting point.

Once it's ready either one of us should be able to rake release to create the gem, I think.

Oh BTW I just released schema_plus 2.0.0.pre3 which uses schema_monkey 1.0, that would be the best one to test with now.

Cheers

[fj]

Thanks! I'm a little slammed and traveling this week but I'll look at it this weekend for sure.

[ronen]

@fj Schema_monkey now inserts itself, no explicit insertion step is necessary. So schema_monkey_rails and schema_monkey_padrino are no longer needed. (I've retired schema_monkey_rails.)

I just released schema_plus 2.0.0.pre8, if you update to that you should be able to remove any insertion stuff in the main code or in rake tasks.

[fj]

Following up on this: I just had an opportunity to upgrade schema_plus, but it's not clear to me how an arbitrary task will get schema_plus mixed into it. For example, is it sufficient to just have require 'schema_plus' in the Rakefile and the necessary methods are inserted?

This does seem to work for my purposes, so I think we can close this if you're in agreement.

[ronen]

Yes, just require 'schema_plus' should be all that's needed. If that's working for you, all's good.

[fj]

:+1: