brendon / acts_as_list

An ActiveRecord plugin for managing lists.
http://brendon.github.io/acts_as_list/
MIT License
2.05k stars 356 forks source link

Issue after upgrading to 0.7.5: No connection pool with id primary found. #214

Closed rdvdijk closed 8 years ago

rdvdijk commented 8 years ago

I'm getting a No connection pool with id primary found error when using ActiveRecord 5.0.0 with acts_as_list 0.7.5. All works fine 0.7.4.

Here is the top of the stack trace:

/Users/roel/.gem/ruby/2.3.1/gems/activerecord-5.0.0/lib/active_record/connection_adapters/abstract/connection_pool.rb:874:in `retrieve_connection': No connection pool with id primary found. (ActiveRecord::ConnectionNotEstablished)
    from /Users/roel/.gem/ruby/2.3.1/gems/activerecord-5.0.0/lib/active_record/connection_handling.rb:128:in `retrieve_connection'
    from /Users/roel/.gem/ruby/2.3.1/gems/activerecord-5.0.0/lib/active_record/connection_handling.rb:91:in `connection'
    from /Users/roel/.gem/ruby/2.3.1/gems/acts_as_list-0.7.5/lib/acts_as_list/active_record/acts/list.rb:77:in `acts_as_list'
    from /Users/roel/code/rust-radio-stream/lib/rust_radio/model/playlist_entry.rb:6:in `<class:PlaylistEntry>'
    ... [SNIP] ...

The code in question looks like this:

require 'acts_as_list'

module RustRadio
  class PlaylistEntry < ActiveRecord::Base

    acts_as_list scope: :playlist

    # other code removed

  end
end

And for the sake of completion, Playlist is:

module RustRadio
  class Playlist < ActiveRecord::Base

    has_many :entries,
      -> { order(:position) },
      class_name: "PlaylistEntry"

    # other code removed 

  end
end
brendon commented 8 years ago

Hi @rdvdijk :) That's an interesting error. Just looking at your code, the only unusual things I see is that you're nesting your classes in a Module, and you're explicitly requiring acts_as_list (you shouldn't need to, it's already injected into ActiveRecord::Base.

If you can experiment with those two aspects and get back to me, that would be a huge help :)

If it's still failing, the next step would be to try to create a failing test case with our test suite. It's very easy to get the test suite up and running, so give this a go and let me know if you get stuck.

rdvdijk commented 8 years ago

Note that this is not a Rails project, but stand-alone usage of ActiveRecord.

Another thing I've just realized, I have this in my Gemfile:

gem 'acts_as_list', "0.7.5", require: false

I think the require: false was a workaround for something I encountered in the past. I'll try and remove the require: false, and remove the require from the the source code.

(I'm currently at work, I'll report back when I get around to testing this tonight.)

rdvdijk commented 8 years ago

I wrote:

I'll try and remove the require: false, and remove the require from the the source code.

That doesn't seem to work either.

If I do not require acts_as_list myself, this is the error message I get (which was to be expected) :

NoMethodError: undefined method `acts_as_list' for #<Class:0x007ffcc298a9b0>
Did you mean?  acts_like?
  /Users/roel/.gem/ruby/2.3.1/gems/activerecord-5.0.0/lib/active_record/dynamic_matchers.rb:21:in `method_missing'
  /Users/roel/code/rust-radio-stream/lib/rust_radio/model/playlist_entry.rb:4:in `<class:PlaylistEntry>'
rdvdijk commented 8 years ago

@brendon I started a small test-project to try and replicate this issue. The solution turned out to be extremely trivial, but interesting nonetheless: acts_as_list needs an established connection to the database when loading the ActiveRecord models.

My old code used acts_as_list 0.2.0 (yeah, that old), and acts_as_list didn't need that connection. Note that this was introduced in 0.7.5, because versions prior to that work just fine without an established connection during model load-time.

brendon commented 8 years ago

@rdvdijk, thanks for digging into that a bit deeper. I wonder what part is requiring the active database connection? @swanandp, do you have any ideas?

I've come across this with the authlogic gem too. Though that's because it enumerates something in the users table to determine certain settings.

At least you've hopefully found the fix for your situation however :)

rdvdijk commented 8 years ago

Line 77 of list.rb is accessing the connection:

quoted_position_column = connection.quote_column_name(configuration[:column])

The quoted_position_column is then used in the class_eval block directly following it.

Looks to me like quoting the column name could probably be postponed and done inside the class_eval block, which would eliminate the need of escaping it during model load-time?

brendon commented 8 years ago

I see what you mean :) The class_eval block has always bugged me, there's probably a cleaner way to do all of that. Perhaps we can make quoted_position_column (at the bottom of the file) a class method and lazily access it in the class methods rather than interpolate it in the class_eval?

rdvdijk commented 8 years ago

@brendon @swanandp I've created pull request #215, which also addresses this issue.

swanandp commented 8 years ago

Thanks @rdvdijk ! I have been meaning to get rid of the class_eval for some time now.