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

Provide an option to skip orm hooks #1668

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cwjenkins
Copy link

@cwjenkins cwjenkins commented Aug 31, 2023

Summary

Currently trying to dockerize an application that contains doorkeeper and upon running assets precompilation it wants a database connection to evaluate database.yml config.
This provides a way to skip the orm hooks.

@nbulaj
Copy link
Member

nbulaj commented Sep 26, 2023

Hey @cwjenkins , thanks for your contribution! It looks a little bit odd for me to introduce such a config option for such case.. What log do you see while compiling the assets in docker ?

@cwjenkins
Copy link
Author

Hey @nbulaj, it attempts to make a database connection causing the compilation step to fail given assets being compiled occurs during CI/CD where no DB connection exists.

@nbulaj
Copy link
Member

nbulaj commented Sep 26, 2023

Will be great to understand at what step it tried to connect to the DB and why it requires that step @cwjenkins . In such case we can think how to make it lazy evaluatable (btw it should be already 🤔 )

@cwjenkins
Copy link
Author

Thanks @nbulaj.
I misspoke, not establishing a connection, but rather evaluating the database.yml config.
If missing the particular environment that its getting deployed too then it will raise.
We could look at adding each env into the DB config (currently they are dynamically added during deploy), but seems odd to need such a req for assets compilation.

gems/activerecord-6.1.7.6/lib/active_record/database_configurations.rb:144:in `resolve'
gems/activerecord-6.1.7.6/lib/active_record/connection_handling.rb:367:in `resolve_config_for_connection'
gems/activerecord-6.1.7.6/lib/active_record/connection_handling.rb:51:in `establish_connection'
gems/activerecord-6.1.7.6/lib/active_record/railtie.rb:222:in `block (2 levels) in <class:Railtie>'
gems/activesupport-6.1.7.6/lib/active_support/lazy_load_hooks.rb:71:in `class_eval'
gems/activesupport-6.1.7.6/lib/active_support/lazy_load_hooks.rb:71:in `block in execute_hook'
gems/activesupport-6.1.7.6/lib/active_support/lazy_load_hooks.rb:61:in `with_execution_control'
gems/activesupport-6.1.7.6/lib/active_support/lazy_load_hooks.rb:66:in `execute_hook'
gems/activesupport-6.1.7.6/lib/active_support/lazy_load_hooks.rb:52:in `block in run_load_hooks'
gems/activesupport-6.1.7.6/lib/active_support/lazy_load_hooks.rb:51:in `each'
gems/activesupport-6.1.7.6/lib/active_support/lazy_load_hooks.rb:51:in `run_load_hooks'
gems/activerecord-6.1.7.6/lib/active_record/base.rb:315:in `<module:ActiveRecord>'
gems/activerecord-6.1.7.6/lib/active_record/base.rb:15:in `<main>'
ruby/site_ruby/2.7.0/rubygems/core_ext/kernel_require.rb:37:in `require'
ruby/site_ruby/2.7.0/rubygems/core_ext/kernel_require.rb:37:in `require'
gems/bootsnap-1.16.0/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:32:in `require'
gems/zeitwerk-2.6.11/lib/zeitwerk/kernel.rb:38:in `require'
gems/doorkeeper-5.6.6/lib/doorkeeper/orm/active_record/access_grant.rb:6:in `<module:Doorkeeper>'
gems/doorkeeper-5.6.6/lib/doorkeeper/orm/active_record/access_grant.rb:5:in `<main>'
lib/ruby/site_ruby/2.7.0/rubygems/core_ext/kernel_require.rb:37:in `require'
lib/ruby/site_ruby/2.7.0/rubygems/core_ext/kernel_require.rb:37:in `require'
gems/bootsnap-1.16.0/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:32:in `require'
gems/zeitwerk-2.6.11/lib/zeitwerk/kernel.rb:38:in `require'
gems/activesupport-6.1.7.6/lib/active_support/inflector/methods.rb:287:in `const_get'
gems/activesupport-6.1.7.6/lib/active_support/inflector/methods.rb:287:in `block in constantize'
gems/activesupport-6.1.7.6/lib/active_support/inflector/methods.rb:283:in `each'
gems/activesupport-6.1.7.6/lib/active_support/inflector/methods.rb:283:in `inject'
gems/activesupport-6.1.7.6/lib/active_support/inflector/methods.rb:283:in `constantize'
gems/activesupport-6.1.7.6/lib/active_support/core_ext/string/inflections.rb:74:in `constantize'
gems/doorkeeper-5.6.6/lib/doorkeeper/config.rb:443:in `access_grant_model'
gems/doorkeeper-5.6.6/lib/doorkeeper/orm/active_record.rb:40:in `initialize_configured_associations'
gems/doorkeeper-5.6.6/lib/doorkeeper/orm/active_record.rb:32:in `run_hooks'
gems/doorkeeper-openid_connect-1.8.6/lib/doorkeeper/openid_connect/orm/active_record.rb:13:in `run_hooks'
gems/doorkeeper-5.6.6/lib/doorkeeper.rb:168:in `run_orm_hooks'

@cwjenkins
Copy link
Author

Hey @nbulaj any interest in providing a way to exclude AR initialization ?

@nbulaj
Copy link
Member

nbulaj commented Oct 9, 2023

Yeah, but I believe it shouldn't be a config option. We have to change autoloading to be fully lazy-evaluatable so it won't affect anything in Docker or in any other env

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

Successfully merging this pull request may close these issues.

2 participants