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

Do not prematurely load ActiveRecord::Base #212

Closed
wants to merge 1 commit into from
Closed

Do not prematurely load ActiveRecord::Base #212

wants to merge 1 commit into from

Conversation

fatkodima
Copy link

While investigating rails/rails#47914, I identified that this gem prematurely loads ActiveRecord::Base and so the application is not properly configured at the end.

I detected a similar problem recently with the other gem - ledermann/unread#131. The inners of the problem are described in the comments to the rails/rails#46567.

@albus522
Copy link
Member

This change doesn't actually work because it requires something external to delayed job to trigger ActiveRecord which is not required to happen. We have already tried and rolled back a different mostly working approach that had other issues.
The problem is that Rail's argument for how you are suppose to hook into ActiveRecord doesn't apply to things like delay job. We aren't augmenting ActiveRecord, we are using it.

@albus522 albus522 closed this Apr 11, 2023
@fatkodima
Copy link
Author

What do you think about doing something like this?

# lib/railtie.rb
class Railtie < Rails::Railtie
  initializer "delayed_job.initialization" do
    ActiveSupport.on_load(:after_initialize) do
      require "activerecord/base"
      require "delayed/backend/active_record"
      Delayed::Worker.backend = :active_record
    end
  end
end
if defined?(Rails)
  require_relative "railtie"
else
  require "delayed/backend/active_record"
  Delayed::Worker.backend = :active_record
end

@albus522
Copy link
Member

An initializer was the previous attempt that didn't work. There were a number of interconnected reasons that broke people's apps and we reverted.

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

2 participants