Queue priority in config

[ryannealmes]

Queue priority is not settable via a config file. This causes complications when you want to set queue priority and leverage ActiveJob in Rails. In order to allow setting queue priority I have created a queue_attributes attribute that falls off the worker. The attribute is then accessed and the priority is set when setting up the job prior to enqueuing.

I have extracted the enqueueing code into it's own class to try and clean things up a little into a JobPreparer for a lack of a better name.

Some side notes it may be better to rather use the queue attribute on the worker, but I thought this was a good start.

This is my first public PR to someone else's repo. I have read the guidelines ... I am happy if it's rejected, but input on where I can improve in future or how I can change things to provide similar functionality would be much appreciated.

Someone also started a thread requesting this functionality.

[sferik]

Can you please rebase from master and fix the outstanding RuboCop style guide violations in your code?

[ryannealmes]

I have rebased from master ... I hope I have done it correctly. Let me know if there is anything I need to fix. There were also a couple of rubocop issues that weren't mine which I included as well.

[sferik]

I hope I have done it correctly.

@ryannealmes This doesn’t look quite right. Try this in your fork:

git remote add upstream git@github.com:collectiveidea/delayed_job.git
git checkout master
git pull upstream master
git checkout queue-priority-in-config
git rebase master
# after each conflict is resolved resolved:
git add .
git rebase --continue
# after all conflicts are resolved:
git push --force origin queue-priority-in-config

Also, ideally all of the changes in your branch would be squashed into a single commit. Let me know if you need instructions on how to do that.

[ryannealmes]

Cool, I think I am there. Let me know what you think. Thanks for the help.

[ryannealmes]

Hey @sferik, is there anything I need to do to get this merged? I know guys are probably pretty busy with other stuff, but I am not really sure if I should just wait until someone says something or if I should actively be chatting to someone. Thanks.

[svoop]

@ryannealmes Thanks for this PR, this is really a crucial bit! (I have to up an app to Rails 4.2 now and without this feature, I'll be forced to switch to another backend.)

@sferik Any chance this will be merged soonish?

[sferik]

Should this have </code> at the end?

[sferik]

Also, just one space after rubocop:disable.

[sferik]

Sorry for the long delay on merging this pull request. Other than the few minor questions/suggestions I just made related to RuboCop, this looks good to me.

I’d like to merge this and release it at the same time as Rails 5 compatibility. Of course, Rails 5 is not yet released but I’m hoping it will come later this year or early 2016.

In the mean time, after this is merged, you’ll be able to use it by specifying a Git source in your Gemfile:

gem 'delayed_job', :git => 'https://github.com/collectiveidea/delayed_job.git'

[svoop]

@sferik @ryannealmes

There might still be a problem lurking. With the version from the queue-priority-in-config branch of ryannealmes/delayed_job, I experience the following behaviour:

# initializers/delayed_job.rb
Delayed::Worker.queue_attributes = [
  { name: :low_priority, priority: 10 }
]

"test".delay(queue: :low_priority).to_s
job = Delayed::Job.last
job.queue      # => "low_priority"   <-- OKAY
job.priority   # => 10               <-- OKAY

UserMailer.password_reset(user).deliver_later(queue: :low_priority)
job = Delayed::Job.last
job.queue      # => "low_priority"   <-- OKAY
job.priority   # => 0                <-- FAIL

[svoop]

@sferik @ryannealmes

I found the cause for the problem mentioned in my previous comment. While the ActiveJob documentation uses symbols for queue names, the actual queue name is converted to a string internally. Therefore, when setting default priorities per queue, you must currently use a string to reference the queue. Now, I won't be the only one tripping over this 😄

I suggest adding a to_s to the priority discovery method in this PR:

Delayed::Backend::JobPreparer#set_priority:

  def set_priority
    options[:priority] ||= Delayed::Worker.default_priority
-   queue_attributes = Delayed::Worker.queue_attributes.select { |queue| queue[:name] == options[:queue] }
+   queue_attributes = Delayed::Worker.queue_attributes.select { |queue| queue[:name].to_s == options[:queue] }
    options[:priority] = queue_attributes.first[:priority] if queue_attributes.any?
  end

[svoop]

@ryannealmes I've added a PR to your fork with this fix and a little addition to the README explaining your feature. The rubocop directive cleanup suggested by @sferik is also included: ryannealmes#2

[ryannealmes]

Thanks for the help @svoop and @sferik . I have merged and rebased @svoop 's changes. Let me know if there is anything else.

[sferik]

Looks good to me! Thanks for the patch!

[ryannealmes]

@sferik , @svoop do you guys know why this PR wouldn't show up in my public contributions? Thanks.

[svoop]

@ryannealmes No idea, after all, collectiveidea/delayed_job is listed as a repo you've contributed to. Maybe a Github glitch.

[ryannealmes]

@svoop I was looking forward to being a contributor :/ At least the functionality is there now :)

[maxjacobson]

Maybe it uses the timestamp of the commit, and the public activity thing doesn't scroll back to September

[sferik]

@ryannealmes It’s because this is a fork and GitHub only counts contributions to the original repo.

[swrobel]

Any chance of getting a release with this change before all of the rails 5 stuff @sferik?

[albus522]

We already have the rails 5 stuff in master. I don't like releasing something against in flux dependencies. Have been bitten by that too much.

[ConfusedVorlon]

fair enough.

I wonder if I'm missing a simple way to get what I want anyway without risking the bleeding edge.

Is there a way to set the priority with a rails4 mailer?

something like:

#doesn't work
MyMailer.signup(message,recipient).deliver_later(priority: 5)

I know that activeMailer supports queues - so default queue priorities will allow

MyMailer.signup(message,recipient).deliver_later(queue: "high_priority")

[albus522]

Be aware I have simplified the configuration structure on this in master and the about to be released gem
https://github.com/collectiveidea/delayed_job#named-queues