Queue order is reversed if passed through CLI

[Drew-Goddyn]

How to reproduce:

1. Pass in 2 queues as a CLI argument: shoryuken start -q queue1 queue2 -C ./shoryuken.yml
2. View the value of Shoryuken.options[:queues] in the worker

Expected output:
We expect the order of the queues to match the order they were passed in. I.E. we expect Shoryuken.options[:queues] to evaluate to [["queue1"],["queue2"]]

Current output:
https://github.com/phstc/shoryuken/blob/master/lib/shoryuken/environment_loader.rb#L84 deletes and then readds each queue to the Shoryuken options resulting in Shoryuken.options[:queues] evaluating to [["queue2"],["queue1"]]

Why is this important?
If you've also defined polling_strategy: StrictPriority within your config file, then the order of the queues passed in through the CLI determines which queue is high and which is low priority. This issue causes the first queue passed in the CLI to be the lowest priority when it should be the highest according to the documentation: https://github.com/phstc/shoryuken/wiki/Polling-strategies

[phstc]

Hi @Drew-Goddyn

Great catch, and thanks for the well detailed explanation.

why merge_cli_defined_queues

The idea behind that method is if you have a shoryuken.yml with:

# shoryuken.yml

queues:
  - [queue1, 8]
  - [queue2, 4]

And you call shoryuken passing in the same queue names with different values:

shoryuken start -q queue1,10 queue2,20 -C shoryuken.yml

Expected behavior

The values passed in the CLI should override the ones configured in shoryuken.yml. queue1 should have priority 10 instead of 8, and queue2 should have priority 20 instead of 3.

Quick fix

Add a .dup here:

cli_defined_queues = options[:queues].to_a.dup

(would you like to create a pull request for it? 🙏)

Possible better fix

☝️that should fix the problem. But while debuggingbinding.pry the problem, it seems merge_cli_defined_queues is no longer necessary, the expected behavior (CLI overrides config) seems to be working even without that hacky merge_cli_defined_queues. So, .dup is still an easy fix, but if you are more interested on researching this, check if merge_cli_defined_queues is still needed, if not ✂️ it.

---

WDYT?

[Drew-Goddyn]

I was actually going to suggest we completely remove merge_cli_defined_queues as well as it no longer seemed necessary in my tests. I have some specs written already so I'll make a PR to remove the method.