iconara / mikka

JRuby wrapper for Akka
107 stars 14 forks source link

Add support for declaring supervision strategy on actors #7

Closed akira closed 12 years ago

akira commented 12 years ago

Hello, I added a patch to support supervision strategy in a ruby-like way, so thought it could be useful, feel free to modify/adapt it.

Description:

Example usage:

  # declared in actor
  def supervisor_strategy 
    @strategy ||= Mikka::OneForOneStrategy.new(10, Mikka::Duration["1 minute"]) do |e|      
      :restart
    end
  end
iconara commented 12 years ago

Thanks a lot for this. I like it. I have done some rough sketches before on a class-level DSL for supervision strategy, but never got around to implementing it, but this is good.

What I was thinking about was something like this:

class SomeActor < Mikka::Actor
  supervisor_strategy :one_for_one, 10, Mikka::Duration['1 minute'] do |e|
    :restart
  end
  # ...

I'm stealing a bit of your version because I can't remember exactly how I planned on doing what you do with the block. What do you think about doing it like that, with a class macro -style DSL? I think it can be added on top of your implementation quite easily. If you would like to implement it I'd be very happy, otherwise I'll merge your version and maybe implement that myself later. Both versions can coexist, I think (the macro could just implement the #supervisor_strategy instance method, so the end result would be the same).

I don't have many tests for Mikka, because it's hard to come up with good tests for something that just adapts another framework, but if you can think of something about this that could be tested it wouldn't hurt.

akira commented 12 years ago

Great, I think the class macro style DSL definitely makes sense. I can probably take a crack at the class level implementation at some point, maybe next week.

As far as tests, it would probably be nice to have tests, but the testkit jar will probably be needed for more complex tests.

iconara commented 12 years ago

Ok, cool. I'll merge this in the meantime.