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

Postpone configuration (database introspection) #264

Closed
Envek opened this issue Apr 14, 2017 · 10 comments
Closed

Postpone configuration (database introspection) #264

Envek opened this issue Apr 14, 2017 · 10 comments

Comments

@Envek
Copy link
Contributor

Envek commented Apr 14, 2017

Please consider to do database introspection not when calling has_closure_tree method (on application code load) but some time later.

I'm using Docker Compose for development and because of that I'm launching container with Rails application immediately after container with database has started. Sometimes has_closure_tree will be invoked and will attempt to access DB to analyze tables before RDBMS is ready to serve clients. So, application will fail to load and application server will be terminated).

Rails itself (without closure_tree) doesn't hit the DB until first request will come, so everything works as expected even if DB was not ready at time of first request (ActiveRecord will analyze tables when DB will be ready).

Also request time (or first model access time) configuration should fix other issues like #214

Thank you for closure_tree!

@mceachen
Copy link
Collaborator

Great idea; it'd be great to get rid of the hackiness around initialization.

Do you know of a reliable loading event I can register with, or would I need to add an initialize call to every method in closure_tree (which would be pretty terrible and error prone).

If you feel up to it, have a swing at a pull request!

@n-rodriguez
Copy link
Contributor

Hi there! Got the same problem here when testing my app with Rails 5.2.0.rc2.

@n-rodriguez
Copy link
Contributor

I could workaround it with monkey patching :

# frozen_string_literal: true

require 'closure_tree/support_flags'
require 'closure_tree/support_attributes'

module MyApp
  module CoreExt
    module ClosureTreePatch

      module SupportFlagsPatch
        def order_is_numeric?
          # return if not connected to DB (on rake db:setup for instannce)
          return false unless ::ActiveRecord::Base.connected?

          # The table might not exist yet (in the case of ActiveRecord::Observer use, see issue 32)
          return false if !order_option? || !model_class.table_exists?
          c = model_class.columns_hash[order_column]
          c && c.type == :integer
        end
      end

      module SupportAttributesPatch
        def quoted_hierarchy_table_name
          # return if not connected to DB (on rake db:setup for instannce)
          return unless ::ActiveRecord::Base.connected?
          connection.quote_table_name hierarchy_table_name
        end
      end

    end
  end
end

unless ClosureTree::SupportFlags.included_modules.include?(MyApp::CoreExt::ClosureTreePatch::SupportFlagsPatch)
  ClosureTree::SupportFlags.send(:prepend, MyApp::CoreExt::ClosureTreePatch::SupportFlagsPatch)
end

unless ClosureTree::SupportAttributes.included_modules.include?(MyApp::CoreExt::ClosureTreePatch::SupportAttributesPatch)
  ClosureTree::SupportAttributes.send(:prepend, MyApp::CoreExt::ClosureTreePatch::SupportAttributesPatch)
end

@mceachen
Copy link
Collaborator

mceachen commented Apr 7, 2018

@n-rodriguez Could you warp this in a PR? That would be great!

Turns out I didn't know about .connected? when I wrote this part. (and it looks like it was added way back in 2.3!)

@n-rodriguez
Copy link
Contributor

well I'm not sure it it's the way to go... For now I try to make the tests pass on my app with Rails 5.2.0.rc.2 ^^

@n-rodriguez
Copy link
Contributor

At least I can say it works for me, but I don't know the method call path so other methods may be impacted like : https://github.com/ClosureTree/closure_tree/blob/master/lib/closure_tree/support_attributes.rb#L63 and others.

@Envek
Copy link
Contributor Author

Envek commented Apr 8, 2018

Also note that there is some official™ way have been discovered with load_schema! class-level method: rails/rails#31681 (comment) (maybe it helps)

@grk grk mentioned this issue Apr 16, 2018
@seuros seuros added this to the 7.0.0 milestone Apr 16, 2018
@n-rodriguez
Copy link
Contributor

n-rodriguez commented Apr 17, 2018

Hi! The PR is here #306 :)
IMHO I don't think load_schema! is the way to go. I've finally make the tests pass on my app and so far it looks good. it's not yet in staging or in production but the CI pass \o/
Actually I remembered I use the same trick in my fork of RoyceRole and I've never had issues with this.

@n-rodriguez
Copy link
Contributor

IMHO I don't think load_schema! is the way to go.

IMHO I DO think load_schema is the way to go. :)

@n-rodriguez
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants