Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

adding Rails::Queueing::Container

This allows us to do:

In your configuration:
Rails.queue[:image_queue] = SomeQueue.new
Rails.queue[:mail_queue]  = SomeQueue.new

In your app code:
Rails.queue[:mail_queue].push MailJob.new

Both jobs pushed to the same default queue
Rails.queue.push DefaultJob.new
Rails.queue[:default].push DefaultJob.new
  • Loading branch information...
commit 3b3ca133071419a42c0b1f55c96fc604ff73f2ac 1 parent fa3c84f
@tenderlove tenderlove authored
View
2  railties/lib/rails/application.rb
@@ -177,7 +177,7 @@ def config #:nodoc:
end
def queue #:nodoc:
- @queue ||= build_queue
+ @queue ||= Queueing::Container.new(build_queue)
end
def build_queue #:nodoc:
View
27 railties/lib/rails/queueing.rb
@@ -1,7 +1,34 @@
require "thread"
+require 'delegate'
module Rails
module Queueing
+ # A container for multiple queues. This class delegates to a default Queue
+ # so that Rails.queue.push and friends will Just Work. To use this class
+ # with multiple queues:
+ #
+ # # In your configuration:
+ # Rails.queue[:image_queue] = SomeQueue.new
+ # Rails.queue[:mail_queue] = SomeQueue.new
+ #
+ # # In your app code:
+ # Rails.queue[:mail_queue].push SomeJob.new
+ #
+ class Container < DelegateClass(::Queue)
+ def initialize(default_queue)
+ @queues = { :default => default_queue }
+ super(default_queue)
+ end
+
+ def [](queue_name)
+ @queues[queue_name]
+ end
+
+ def []=(queue_name, queue)
+ @queues[queue_name] = queue
+ end
+ end
+
# A Queue that simply inherits from STDLIB's Queue. Everytime this
# queue is used, Rails automatically sets up a ThreadedConsumer
# to consume it.
View
10 railties/test/application/queue_test.rb
@@ -20,14 +20,14 @@ def app_const
test "the queue is a TestQueue in test mode" do
app("test")
- assert_kind_of Rails::Queueing::TestQueue, Rails.application.queue
- assert_kind_of Rails::Queueing::TestQueue, Rails.queue
+ assert_kind_of Rails::Queueing::TestQueue, Rails.application.queue[:default]
+ assert_kind_of Rails::Queueing::TestQueue, Rails.queue[:default]
end
test "the queue is a Queue in development mode" do
app("development")
- assert_kind_of Rails::Queueing::Queue, Rails.application.queue
- assert_kind_of Rails::Queueing::Queue, Rails.queue
+ assert_kind_of Rails::Queueing::Queue, Rails.application.queue[:default]
+ assert_kind_of Rails::Queueing::Queue, Rails.queue[:default]
end
class ThreadTrackingJob
@@ -144,7 +144,7 @@ def push(job)
test "a custom queue implementation can be provided" do
setup_custom_queue
- assert_kind_of MyQueue, Rails.queue
+ assert_kind_of MyQueue, Rails.queue[:default]
job = Struct.new(:id, :ran) do
def run
View
30 railties/test/queueing/container_test.rb
@@ -0,0 +1,30 @@
+require 'abstract_unit'
+require 'rails/queueing'
+
+module Rails
+ module Queueing
+ class ContainerTest < ActiveSupport::TestCase
+ def test_delegates_to_default
+ q = Queue.new
+ container = Container.new q
+ job = Object.new
+
+ container.push job
+ assert_equal job, q.pop
+ end
+
+ def test_access_default
+ q = Queue.new
+ container = Container.new q
+ assert_equal q, container[:default]
+ end
+
+ def test_assign_queue
+ container = Container.new Object.new
+ q = Object.new
+ container[:foo] = q
+ assert_equal q, container[:foo]
+ end
+ end
+ end
+end

9 comments on commit 3b3ca13

@dmathieu
Collaborator

Perhaps, if the asked queue isn't defined, Rails::Queuing::Container could fallback on the default one ?

@tenderlove
Owner

@dmathieu ya, look at the Container tests. It delegates to the default queue, which is constructed via the configuration. We generate configuration that specifies a default. :-)

@dmathieu
Collaborator

I mean: if I do Rails.queue[:mail_queue].push MailJob.new, but Rails.queue[:mail_queue] is nil, it wouldn't work.
It could fallback to Rails.queue[:default].

This would allow, for example, engines to use the default queue and their tasks to be extractable into their own.

@josevalim
Owner

@dmathieu :-1: for this behavior. I would prefer for it to explode, users should opt-in for such behavior easily by doing Rails.queue[:mail_queue] = Rails.queue[:default] or something like that.

@dmathieu
Collaborator

If I still wasn't explicit enough: 9cc49763934a2ec3016b5bb882c3b8efd2265376
I can either submit a pull request or just forget it :)

@androbtech

I agree with @josevalim

Thinking about your idea @dmathieu , the only thing that bothers me is that it defeats the purpose of having a container of queues. Engine users should be well aware of the intent and requirement of the queue container. Your code, to my interpretation, would be like treating this edge case as "just throw it down there, we don't care".

@rafaelfranca

@dmathieu :-1: for this behavior too. Add this means that one typo can add a job in another queue, and this to me is not expected behavior.

@tenderlove
Owner

@dmathieu I am also :-1:. I want my code to explode if the queue is missing.

Please sign in to comment.
Something went wrong with that request. Please try again.