Be able to access headers in work()

[maxigs]

For some cases, that i would like to use sneakers for, i need access to the AMQP headers in the work() method.

Should i just have do_work() hand it through to work()?
This would make it necessary to have work() accept more than one parameter in the future, but i think its the easiest way without changing much else.

[jondot]

Hi Benjamin,

The headers were hidden from the developer in order to avoid exposing AMQP internals, so that a developer will work in the mind frame of what he/she already knows with libraries like Sidekiq, Resque and delayed_job.

I think it should not be a problem to expose them - however I would like to preserve simplicity. I would be happy to hear what kind of example API you would expect that would serve you best and still be simple.

Thanks

[maxigs]

Hi Dotan, that was quick ;-)

As the AMQP guide state its good design to use the AMQP metadata headers (http://rubybunny.info/articles/exchanges.html#message_metadata). Usually i keep the user_id and other related information to the actual data in there in the headers key.
So far this would be easily accessible in the work method, for simplicity i'd just hand it through for now making it a second parameter.

def work(msg, headers) # not hdr
  ...

The higher up message metadata (routing_key, reply_to, ..) are very useful also sometimes. but it would complicate the access to the headers (2 levels).

To keep the interface nice would keep those out of the work() method and instead add a nice interface to make use of them: For example reply() to publish a message to the supplied reply_to key.
For this to be possible we would need a task-class wrapping the work() method to keep the information and state.
Adding this would break the current design a bit so i'm not quite sure if you want to go that way.

Kinda like this:

def do_work(hdr, props, msg, handler)
  ...
  # some 'magic' to determine the class
  task = SomeTask.new(hdr, props, msg)
  task.call
  res = task.state
  ...
end

class Sneakers::Task
  def initialize(hdr, props, msg)
    @state = :new 
    @props = props
    @headers = props[:headers]
    ...
  end

  def ack!
    @state = :ack
  end

  def reply(msg)
    publish(msg, @props[:reply_to])
  end
  ...
end

class SomeTask
  include Sneakers::Task

  def call
     # this would replace the work() in Worker
     # do some work
     reply('some result')
     ack!
  end
end

This is pretty much how i usually handle this part, and i was just about to start hacking it into sneakers so i can test it a bit with existing Tasks

[maxigs]

Since i anyway tried it, here is a (running, minimal) example:

maxigs@12b40e7

for this code

class SomeTasksWorker
  include Sneakers::Worker
  from_queue :some_tasks

  class Task
    include Sneakers::Task
    def call
      ack!
    end
  end
end

[jondot]

wow, thanks! I will look into it once I have the time to experiment. I know this might be risking premature optimization - but with the original idea I tried not to create "holder" objects in order to reduce GC overhead, this is why I didn't have the "task" abstraction. However this looks so clean I'm wondering if we can offer both now.
In any case I hope to get into it once I have a good chunk of clean time coming up :)

[maxigs]

We could just make it optional, like so:

if self.respond_to?(:work)
  work(..)
elsif defined?((self.class.name + '::Task').constantize)
  klass = (self.class.name + '::Task').constantize
  task = klass.new(...)
  task.call
  ...
else
  raise 'no work() method or task class'
end

That would not break the old pattern, but allow for the new one to just add more functionality.

[maxigs]

The API i use now, with a bit less magic:

class SomeWorker
  include Sneakers::Worker
  from_queue :tasks, {
    ...
    task_class:        SomeWorkerTask
  }
end

class SomeWorkerTask
  def call
    ...
    ack!
  end
end

Rails did not really like the nested classes (autoload magic), and i think explicitly putting it as a setting is nicer anyway if there is more options.

The change in worker.rb is also really straight forward for this:

res = if @opts[:task_class]
  task = @opts[:task_class].new(msg: msg,...)
  task.call
  task.status
else
  work(msg)
end

While it allows to add whatever additional data to the Task instance. Sorry its not a pull request, but its so simple i think its ok ;)

[jondot]

Hey Benjamin, sorry for the blackout, had some personal matters. Trying to allocate time to work on Sneakers issues now 👍

[jondot]

So I gave it some research, and I may be wrong, and I wouldn't be surprise if I am, but generating more objects would generate a GC overhead we can do away with.
How about taking your initial suggestion for detecting a method early on, and then using it instead?

Let's say something like this

class MyWorker
  include Sneakers::Worker
  from_queue :foo

  def work_with_params(msg, headers, params)
  end
end

Then, Sneakers will have to detect which methods the end-user implemented via your respond_to? suggestion (but only once, on initialization time) and cache the result.
All future calls will go to work_with_params.

What do you think?

[jondot]

you're welcome to test the new branch, there's working tests for example of a worker and usage :)

[maxigs]

Didn't test it yet but the interface looks fine. If needed everyone can still initialise a task class within the work_with_params method in his code as long as that one as access to it.

I will definitely keep using "task-classes" as they make testing the code much easier.

[jondot]

Thanks, Benjamin, I agree your way of doing things is more encapsulated and cleaner code. I've merged the with_params variant and hope it can support both your use case and others.
Pushed as v0.1.0.pre for early adopters. Install with gem install sneakers --pre

[simonmorley]

👍 Thanks for this :)

[alilland]

@jondot is this interface still available? I came across this when googling sneakers and trying to access headers

[michaelklishin]

@alilland something like this?

[alilland]

@michaelklishin this is perfect, solves half my issue, how can I access headers within my def work(msg); end method?

I am desperately trying to find a way to track how many times a message is requeued