search

salesforce / storm-dynamic-spout

A framework for building spouts for Apache Storm and a Kafka based spout for dynamically skipping messages to be processed later.
BSD 3-Clause "New" or "Revised" License
40 stars 13 forks source link

Remove TopologyContext from VirtualSpout #76

Closed stanlemon closed 6 years ago

stanlemon commented 6 years ago

@Crim this is a quick, can I make it compile, type of pass. The tests are broken right now, but this shows you what it'd look like to remove TopologyContext from the VirtualSpout. I kind of like this because it minimizes what Storm specific details leak up from the DynamicSpout.

Crim commented 6 years ago

Seems good. I wonder if we should expand the ConsumerPeerContext object to include the component name. As far as I know thats the only other potentially relevant info we might need.

Crim commented 6 years ago

Yea, probably go ahead with this, I'd add the component id/name (spout id) to the object so thats available.

stanlemon commented 6 years ago

@Crim I rebased this and made some changes to fix tests. The build on this one should be passing now, can you re-review so I can merge it in? One thing I noted on this... I wanted to standardize creation of VirtualSpouts inside of VirtualSpoutTest and that proved to be difficult because of the mockFactoryManager instance. I wonder if we should try to overhaul this test to use the concrete mock instances we've made?