Make sidekiq queue configurable

[averell23]

This allows users to configure wich sidekiq queue to use,
so that the default job/strategy can be used with existing
setups.

[pyromaniac]

Hey, can you add a couple of specs here please?

[averell23]

@pyromaniac I must obviously fix the existing tests.
I can also add a little one for the setting - the change is really minor, and the default behaviour is completely unchanged to what it does now.

[pyromaniac]

Hm, why can't we simply do it like this? #437

[pyromaniac]

I mean, why can't you do like Chewy::Strategy::Sidekiq::Worker.sidekiq_options(queue: :whatever) ?

[averell23]

In that case I'd have to make sure that the option-setting code is run after after any initializer that sets the queue, and the queue cannot be modified at runtime.

With my version, it just fetches the queue name at the time when it enqueues the job.

[pyromaniac]

This is awesome. Let's merge it then. I would ask you to do the same for all of the strategies, but it would be too shameless, right? :) Can you fix one small issue please only?

[pyromaniac]

Let's maybe Chewy.settings return some default value?

[averell23]

Ummm... I'm not sure how to do that. Chewy.settings is a simple hash, and it can (and is, in the tests) be directly assigned to.

So while I can put a default somewhere in the Config class, I cannot make sure that the value does always exist. So we'd still need the fallback in this place...

[pyromaniac]

Also, maybe a couple of specs would be perfect here

[averell23]

@pyromaniac I added a little test piece, it'd be great if this could be merged. I'm not sure what you meant by "the same for all other strategies - the sidekiq queue setting obviously only makes sense for sidekiq.

Or did you meant "make all other strategies configurable"? -> I have no idea what configuration the other strategies would need...

[averell23]

@pyromaniac do you have any idea why the build fails? It doesn't seem related to my change...

[pyromaniac]

I meant "make all other strategies configurable", but don't worry, agree with you. As for CI -

 Elasticsearch::Transport::Transport::Errors::BadRequest:
       [400] {"error":"RemoteTransportException[[node-2][inet[/172.17.0.13:9301]][indices:data/write/index]]; nested: InvalidTypeNameException[mapping type name [_delete_by_query] can't start with '_']; ","status":400}

It is really strange, can you rebase on master? If will not help - just remove _ there

[pyromaniac]

Got it now. Lock the elasticsearch-transport on earlier version, the latest one is broken somehow

[averell23]

@pyromaniac - do I need to do anything, or will you take care of it?

[pyromaniac]

Fix specs please by locking es gem version and I'll merge it.

[pyromaniac]

Try to merge this and take a look on the result :)

[pyromaniac]

Thanks for the PR, seems like after master fixes it was merged without causing any troubles.