nathanmarz/storm — pull request #748 by danehammer (2013-11-17 21:28:08+00:00)
Title: Adds worker lifecycle hook

This is to address #155.

The submitted changes follow the discussion on that issue, with one change: IWorkerHook#start takes a WorkerTopologyContext instead of a TopologyContext, because the code that creates the latter today is in the context of starting a task (and thus its constructor takes a taskId, for example).

I have manually tested the changes by adding an impl of IWorkerHook in a local instance of storm-starter and seeing that it gets the appropriate data passed in at startup. That was by running the build release script, and starting the daemons locally and submitting my locally changed storm-starter topology to that "cluster". I think there is code I haven't identified yet that keeps it from being called when I submit locally.

As far as automated tests, I tested my changes to ThriftTopologyUtils, but I haven't finished testing worker.clj. I had issues with creating a testing implementation of IWorkerHook as it needs to get serialized into the topology thrift object, and upon deserialization, Clojure didn't know what to make of it. So currently I have a java test impl, but I don't want to be the first one to use java files in testing.

So it's very close, I would feel more comfortable merging it after we prove a non-trivial use case like Guice, but I wanted to get it front of some of the contributors' eyes before I did all that.