bensheldon / good_job

Multithreaded, Postgres-based, Active Job backend for Ruby on Rails.
https://goodjob-demo.herokuapp.com/
MIT License
2.67k stars 199 forks source link

Using the `async` worker results in `ActiveModel::UnknownAttributeError unknown attribute 'create_with_advisory_lock' for GoodJob::Job`. #290

Closed patch0 closed 3 years ago

patch0 commented 3 years ago

Hello!

I'm working towards replacing redis/sidekiq in a project, and I've come unstuck during testing -- the existing suite fails. Partly this is because GoodJob doesn't (yet) just drop in as an ActiveJob test queue replacement. So I decided to play around with the executors to see if one would fit the bill, but I hit an unexpected error within GoodJob itself.

When I set the executor to async, I get an ActiveModel::UnknownAttributeError unknown attribute 'create_with_advisory_lock' for GoodJob::Job when a new job is queued.

I added byebug to line 222 if job.rb to take a look at what was going on.

When I set the executor to async_server, create_with_advisory_lock is defined in Lockable, as expected.

[219, 228] in /app/vendor/good_job/lib/good_job/job.rb
   219:           DEPRECATION
   220:         end
   221:
   222:         byebug
   223:
=> 224:         good_job = GoodJob::Job.new(**good_job_args)
   225:
   226:         instrument_payload[:good_job] = good_job
   227:
   228:         good_job.save!
(byebug) GoodJob::Job.new.method(:create_with_advisory_lock).source_location
["/app/vendor/good_job/lib/good_job/lockable.rb", 121]
(byebug)

But when I use async, the method is missing:

(byebug) GoodJob::Job.new.method(:create_with_advisory_lock).source_location
*** NameError Exception: undefined method `create_with_advisory_lock' for class `#<Class:0x000055cac0251300>'

nil

but the Lockable module appears to be included

(byebug) GoodJob::Lockable <=> GoodJob::Job
1

So I'm not certain what is happening. Any ideas? :)

bensheldon commented 3 years ago

Hi @patch0 thanks for opening up an issue. That's definitely unexpected that GoodJob::Job#create_with_advisory_lock is undefined; there's nothing special about that method https://github.com/bensheldon/good_job/blob/410c70ee1430f00c73786c8073c7bd1d3f9041ed/lib/good_job/lockable.rb#L121-L123

Would you be able to create a minimal Rails app that reproduces the issue? :async/:async_server can be delicate with autoloading and gems that influence the load order.

And I'm also curious about your interest in ActiveJob test replacement. Could you say more about your hopes/expectations? I haven't explored that much.

This is the rspec support file I use to separate out tests in which I'm explicitly using active-job assertions (e.g. expect(ExampleJob).to have_been_enqueued):

# spec/support/active_job.rb

RSpec.configure do |config|
  config.include ActiveJob::TestHelper

  # Rails 6 inconsistently overrides ActiveJob queue_adapter setting with TestAdapter
  # https://github.com/rails/rails/issues/37270
  config.around do |example|
    if example.metadata[:active_job]
      original_queue_adapter = ActiveJob::Base.queue_adapter
      ActiveJob::Base.queue_adapter = :test
      example.run
      clear_enqueued_jobs
      clear_performed_jobs
      ActiveJob::Base.queue_adapter = original_queue_adapter
    else
      ActiveJob::Base.queue_adapter = :good_job
      example.run
    end
  end

  config.before do |_example|
    (ActiveJob::Base.descendants + [ActiveJob::Base]).each(&:disable_test_adapter)
  end
end
patch0 commented 3 years ago

Thanks for your reply. I've created a noddy application, and cannot replicate the error. Sigh.

That's definitely unexpected that GoodJob::Job#create_with_advisory_lock is undefined; there's nothing special about that method

Indeed :) I have zero idea why it is behaving like this. It is even reporting that the module has been included at that point!

Regards test integration, I was kind of expecting things to "just work", i.e. behave in the same fashion as they did with the ActiveJob test adaptor. I'd not really put a lot of thought into it, but that snippet looks really helpful. I'll give it a go tomorrow, and report back, probably opening a PR on the README, if that suits.

patch0 commented 3 years ago

Yeah, not sure in the end about that exception. All very strange. I'll close this for now. Thanks for your help @bensheldon