Allow a worker to be used for multiple queues. #153
Conversation
| expect(GlobalDefaultsTestWorker.get_shoryuken_options['batch']).to eq false | ||
| it "still contains configuration not explicitly changed" do | ||
| subject | ||
| expect(dummy_worker.get_shoryuken_options["queues"]).to include("randomqueues") |
There was a problem hiding this comment.
Prefer single-quoted strings when you don't need string interpolation or special symbols.
|
Could we change the hound configuration to not care about meaningless things? |
|
Failure unrelated. |
|
@phstc Any comment on this one? |
|
@Senjai I still need to test it locally. But currently shoryuken supports multiple queues for the same worker: MyWorker.perform_async('Pablo', queue: 'important') # will queue to the 'important' queueHow the shoryuken.yml would look like with your change? Your change is to define multiple queues in there? Could you send me a code snippet showing how to use your changes?
I prefer to keep the code base concise, keeping the same formatting for all code. The motivation on that is to avoid distractions while maintaining the code. Could you review those houndci feedbacks? |
|
@phstc I don't use the workers like that. Our worker application runs standalone, and doesn't run with rails. class MyWorker
include Shoryuken::Worker
shoryuken_options queues: ['queue1', 'queue2']
endThis was previously not supported. Shoryuken.yml would not change as it defines what queues to listen to, not what workers to use. This simply registers the worker twice, once for each queue.
This goes against the grain of most style guides, and the argument of single quotes being more performant is a moot one. From the github style guide:
In CanCanCan I don't really care which literal contributors use, I'd rather have the contributor focus on providing good solutions rather than worry about what quotes they use. If you feel strongly about it, I can change it for this PR. |
|
@Senjai I see your use case, I will test it locally this week, tks 🍻
My motivation on single vs double quotes is not about performance, it's more about being consistent, and to do that, we need to pick one style:
I chose Option A.
Could you? ❤️ |
This allows a worker to be used for more than one queue. Ocassionaly, logic for several queues is the same, creating a class for each queue is cumbersome and can be avoided.
Senjai
force-pushed
the
multiple_queues
branch
from
2bf77cf
to
db63302
Compare
|
|
||
| shoryuken_options auto_delete: true | ||
| it 'overrides default configuration' do | ||
| expect{subject}.to change{dummy_worker.get_shoryuken_options['auto_delete']}. |
There was a problem hiding this comment.
Space missing to the left of {.
Space missing inside {.
Space missing inside }.
|
|
||
| it "registers the result of a block queue" do | ||
| expect(Shoryuken).to receive(:register_worker). | ||
| with("block_queue", dummy_worker) |
There was a problem hiding this comment.
Prefer single-quoted strings when you don't need string interpolation or special symbols.
|
@phstc Bumped |
| end | ||
|
|
||
|
|
|
@Senjai I still think that multiple queues per worker is an edge case, for instance Sidekiq does not support it. So I wouldn't have While I'm still thinking about it, I've also created a new PR #164 which allows |
This allows workers to be used for multiple queues. There may be several queues from several applications, where the process logic is 100% the same. To handle this currently you need one class per queue which violates dry if we could simply have Shoryuken use the same class for each queue.
An example of where this is a requirement would be a failures interface for several applications based on SQS. Each application has its' own dead letter queue, but you can use a common interface to represent and store failures from each dead letter queue.
Additionally, if you work with queues outside of your codebase, the segregation may work for a third party, but may be indifferent in your application, so processing them as separate flows would not make sense.
This deprecates the queue parameter in favour of a queues parameter and allows for a soft cutover to the new logic.