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
41 stars 13 forks source link

Update dependencies and cleanup #111

Closed stanlemon closed 6 years ago

stanlemon commented 6 years ago

This pull request does:

This pull request does NOT:

I will open a followup PR with some of the does not items, this one has gotten too big already.

Crim commented 6 years ago

My only concerns are on the tests where .close() calls got moved out of finally blocks. Those took me ages to track down spots where .close() wasn't being called correctly leaving zombie processes running all over. We should just double check that even when the assertions fail, that close() gets called.

Should be simple to verify by just adjusting one of them to not throw the expected exception and see what happens.

stanlemon commented 6 years ago

Good catch on those finally blocks, I think I got those all back in now and addressed the other feedback as well.