when retrieving queue name, check for 'queue_name' which is how ActiveJob defines queue names

[josh-m-sharpe]

This fixes an issue when queueing jobs from resque-scheduler's Scheudle page. Without this change, resque is unable to identify the queue to drop the job on.

ActiveJob details:

https://api.rubyonrails.org/classes/ActiveJob/QueueName/ClassMethods.html#method-i-queue_as
https://github.com/rails/rails/blob/157920aead96865e3135f496c09ace607d5620dc/activejob/lib/active_job/queue_name.rb#L41
https://github.com/rails/rails/blob/157920aead96865e3135f496c09ace607d5620dc/activejob/lib/active_job/queue_name.rb#L62

[rafaelfranca]

queue_as is how Active Job defines the queue, not how you get the value. Should not this being calling queued_name?

But either way, Resque should not know anything about Active Job.

Why do you think this change is necessary? The Active Job adapter already set the correct queue name to Resque. https://github.com/rails/rails/blob/855c9897eb98ea4d77ec5036d15cdd1764698838/activejob/lib/active_job/queue_adapters/resque_adapter.rb#L33

[josh-m-sharpe]

Branch updated to use queue_name.

I needed this to allow resque-scheduler to correctly queue ActiveJobs: https://github.com/resque/resque-scheduler/blob/master/lib/resque/scheduler/server.rb#L149 - that plugin uses Resque.queue_from_class in a couple other places as well.

I suspect this fix is probably still necessary in this context given that resque-scheduler is a plugin for resque and not ActiveJob... probably??

[josh-m-sharpe]

Yea, I think the issue (which has nothing to do with resque-scheudler) is that Resque.queue_from_class returns false for ActiveJob jobs. The expectation below would be 'default'

$ cat app/jobs/standard_job.rb
class StandardJob < ApplicationJob
  queue_as :default

  def perform(*args)
    sleep 0.1
  end
end

$ cat app/jobs/application_job.rb
class ApplicationJob < ActiveJob::Base
end

$ rc
Loading development environment (Rails 6.0.2.2)
[1] pry(main)> Resque.queue_from_class(StandardJob)
=> false

[rafaelfranca]

That makes sense. That happen because resque doesn't, and should not, know about Active Job. So Resque.queue_from_class(StandardJob) should return false.

Also, we can't determine the queue name for a subclass of ActiveJob::Base using the class. We need to use the instance.

That being said, in order to expose the queue for a job we will need to make significant changes to the Resque plugin in Active Job to wrap each job in a class like we do in the Sidekiq adapter.

[josh-m-sharpe]

Also, we can't determine the queue name for a subclass of ActiveJob::Base using the class. We need to use the instance.

This PR is doing: klass.new.queue_name so I believe it is pulling the correct queue name.

Apologies, but I'm not sure I follow what you are saying here:

That being said, in order to expose the queue for a job we will need to make significant changes to the Resque plugin in Active Job to wrap each job in a class like we do in the Sidekiq adapter.

[iloveitaly]

@josh-m-sharpe Although I agree with rafaelfranca in principle—that resque shouldn't know about ActiveJob—I think it's a better UX to add some light bindings / support for ActiveJob given that's a very popular pattern.

@rafaelfranca Could you explain further why "we will need to make significant changes to the Resque plugin in Active Job"? I'm not seeing any issues here, but I don't have as much experience with ActiveJob.

[iloveitaly]

You could add more inline documentation here about what cases these different lines are handling? Even just a link to this PR in code would be helpful.

[iloveitaly]

@josh-m-sharpe quick reminder here! Would love to tidy up this PR and get it merged.

[josh-m-sharpe]

Feel free to tidy it up and get it merged!