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

Fix Rails autoloading #172

Merged
merged 2 commits into from Nov 8, 2019
Merged

Fix Rails autoloading #172

merged 2 commits into from Nov 8, 2019

Conversation

p8
Copy link
Contributor

@p8 p8 commented Jul 9, 2019

Delay initializing of backend until all initializers have run when using Rails.

As mentioned in #106 (comment) initializing ActiveRecord after this gem is loaded prevents problems with Rails.application.config.active_record.belongs_to_required_by_default.
But instead of using on_load(:active_record) like in #106 this use the on_load(:after_initialize) hook. on_load(:after_initialize) always runs but after Rails initializes so it won't cause problems with rake tasks.
See: https://guides.rubyonrails.org/engines.html#configuration-hooks

require "delayed/backend/active_record" has been removed since it's already called in Delayed::Worker.Backend=

@p8
Copy link
Contributor Author

p8 commented Jul 9, 2019

The build seems to fail for unrelated reasons.

@p8
Copy link
Contributor Author

p8 commented Jul 17, 2019

I've rebased against #173 to get the build green.

@p8
Copy link
Contributor Author

p8 commented Sep 27, 2019

@albus522 I've been running this patch on some production apps without any problems. Anything I can do to get this merged?

@albus522
Copy link
Member

Can you hunt down when ActiveSupport.on_load(:after_initialize) {} started being preferred over config.after_initialize {} to make sure we don't need a compatibility layer? We still support all the way back to ActiveRecord 3.0 as we haven't had a need to break that support.

@p8
Copy link
Contributor Author

p8 commented Oct 1, 2019

Ok, so config.after_initialize {} has been supported for almost 10 years.
https://github.com/rails/rails/blame/98a57aa5f610bc66af31af409c72173cdeeb3c9e/railties/lib/rails/railtie/configuration.rb#L70
And it's still the recommended usage: https://api.rubyonrails.org/classes/Rails/Railtie.html (see Configuration)
I'll change the pull request to use config.after_initialize {} with a Railtie instead.

Explicitly requiring ActiveRecord models in
`lib/delayed_job_active_record.rb` may result in losing custom
configurations that are set in config/initializers on Rails.

Let' use Railtie and ActiveSupport.on_load to set it up so that
Delayed::Backend::ActiveRecord::Job model will respect arbitrary
configs.
@p8
Copy link
Contributor Author

p8 commented Oct 1, 2019

@albus522 I've pulled in #106 and added a commit to use config.after_initialize {}

Delay initializing of backend until all initializers have run.
@p8
Copy link
Contributor Author

p8 commented Oct 1, 2019

The build fails for unrelated reasons.

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.

None yet

3 participants