Added ability to customize sidekiq_options in listeners

[k-rudy]

Hey guys,

This PR adds support of custom sidekiq_options when registering a listener.
In order to work it requires few changes in Wisper krisleech/wisper#159.

It supports all existing configurations + adds ability to pass additional options via the following syntax:

cancel_order.subscribe(OrderNotifier.new, async: { queue: 'custom', retry: false })

Decreased coverage is expected as one of the specs is suspended until wisper PR is merged. After wisper part is merged we'll need to remove this line https://github.com/krisleech/wisper-sidekiq/pull/13/files#diff-078c1c61de993e867792139abaaba7d3R42 that will enable the spec for the new syntax.

JRuby build is failing for unknown reason. I wish I could ssh to container to debug it but have no access. Locally tests work fine on this JRuby version though. Appreciate any ideas. Also @krisleech if you could restart the master branch build (it was not run for 2 years) to make sure it's failing definitely because of the changes in this PR and not because of something else.

Thanks for consideration and appreciate your thoughts.

[coveralls]

Coverage decreased (-10.3%) to 89.655% when pulling 26d588b on Profinda:master into fc5a529 on krisleech:master.

[rromanchuk]

@k-rudy thanks for doing this, deploying your fork now

[krisleech]

what are these dependencies used for?

[k-rudy]

they are already used by the gem. Nothing new was added. When not frozen - they are resolved to higher versions by bundler and as a result build failed for earlier ruby versions. See https://travis-ci.org/krisleech/wisper-sidekiq/builds/214653091 for details

[krisleech]

Ah okay, I guess they might not be needed since updated the Ruby versions on CI.

[k-rudy]

makes sense, will remove those changes then

[krisleech]

I'm definitely open to these changes:

module Wisper
    class SidekiqBroadcaster
 +    attr_reader :options
 +
 +    def initialize(options = {})
 +      @options = options
 +    end
 +
      def broadcast(subscriber, publisher, event, args)
 -      subscriber.delay.public_send(event, *args)
 +      subscriber.delay(options).public_send(event, *args)
      end

But I'm not convinced that we need to change Wisper, as per the discussion here: krisleech/wisper#159

[krisleech]

If you merge master (which I just updated) CI should pass :)

[coveralls]

Coverage decreased (-9.6%) to 88.136% when pulling 23a4b9c on Profinda:master into 712d5a1 on krisleech:master.

[krisleech]

Any progress on this? Thanks.

[k-rudy]

@krisleech it still waits for krisleech/wisper#159 to be merged.

[krisleech]

Closing in favor of #17

[k-rudy]

@krisleech what's the example of defining options. From the README it's not straightforward:

In order to define custom sidekiq_options you can add sidekiq_options class method in your subscriber definition - those options will be passed to Sidekiq's set method just before scheduling the asynchronous worker.

It it like this:

WisperSubscriber.subscribe(UserListener, async: true, sidekiq_options: { queue: 'custom' })

?

[rwojsznis]

@k-rudy please take a look at - https://github.com/krisleech/wisper-sidekiq/pull/17/files#diff-2d6df3b20624fe23ec544f0a2cdecd1bR21 - you basically define class method on your listener class; feel free to improve readme via PR! (it was my doing - the change, but right now I'm pretty occupied) cheers! 🍻

[k-rudy]

@emq got it, thanks