PRX / announce

Announce is a ruby gem for pubsub of messages
MIT License
3 stars 1 forks source link

Seems that announce only runs on ActiveJob, but is that mandatory? #13

Open agustin opened 8 years ago

agustin commented 8 years ago

Hi guys! me again, when I tried to create a subscriber that does not inherit from ActiveJob the gem seems that doesn't like it. For example:

class TestJob
  include Announce::Subscriber
  subscribe_to :subject, [:topic]

  def receive_subject_topic(data)
    puts "data: #{data}"
    puts "subject: #{subject}"
    puts "action: #{action}"
    puts "message: #{message.inspect}"
  end
end

So when module Subscriber call the delegate_event(*args)(L19) passes not a single argument but instead 2, the sqs_msg and the body that when using ActiveJob the ShoryukenAdapter::AnnounceWorker class received and passed only the body.

My questions is do you want me to add a PR with this changes or this is moving out of the vision or idea that you had for the gem, in that case I understand it.

Please let me know in order to know how to proceed Thanks!

kookster commented 8 years ago

Shoryuken workers should receive those 2 args; you have to use shoryuken if not activejob, right?

For us, we only use announce with activejob.

On Oct 26, 2016, at 5:06 PM, olgalover notifications@github.com wrote:

Hi guys! me again, when I tried to create a subscriber that does not inherit from ActiveJob the gem seems that doesn't like it. For example:

class TestJob include Announce::Subscriber subscribe_to :subject, [:topic]

def receive_subject_topic(data) puts "data: #{data}" puts "subject: #{subject}" puts "action: #{action}" puts "message: #{message.inspect}" end end So when module Subscriber call the delegate_event(*args)(L19) passes not a single argument but instead 2, the sqs_msg and the body that when using ActiveJob the ShoryukenAdapter::AnnounceWorker class received and passed only the body.

My questions is do you want me to add a PR with this changes or this is moving out of the vision or idea that you had for the gem, in that case I understand it.

Please let me know in order to know how to proceed Thanks!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

agustin commented 8 years ago

@kookster yeap because shoryuken needs those arguments but the Announce::Subscriber module is the one defining the perform and delegate_event and this last one is only accepts 1 argument I added a tweak to the current opened PR showing it

I can define the delegate_event on the class itself but feels that is against the purpose of having a gem with the module, WDYT?

kookster commented 8 years ago

I see what you did, I think that might be fine, but I'm wishing I had a non-activeJob test project to verify - I may have to make one.