GitHub user krichter722 opened a pull request:
https://github.com/apache/storm/pull/2598
storm-kafka: added ExponentialBackoffMsgRetryManagerSpoutConfig
Only `ExponentialBackoffMsgRetryManager` uses
`SpoutConfig.retryInitialDelayMs`, `retryDelayMultiplier`, `retryDelayMaxMs`
and `retryLimit` which means that those properties should be moved to a
subclass `ExponentialBackoffMsgRetryManagerSpoutConfig` and the type be
enforced through class parameters. Since `ExponentialBackoffMsgRetryManager` is
the only `FailedMsgRetryManager` is the only implementation there's no need to
make excessive use of class parameter since that flexibility is YAGNI.
Furthermore the properties are declared redundantly in
`ExponentialBackoffMsgRetryManager` and `SpoutConfig`.
I don't seem to figure out how to find which new checkstyle violations the
PR introduces, so I had to guess which changes caused the temporary increase in
violations.
The larger portion of commented out code is left there since checkstyle
value for NumberParameter of 7 seems overkill and at least constructors should
be allowed to be much longer, we should talk about that on the mailing list.
I got the note that apache-kafka might be removed from the project. I'm
working with this code and want to share my work. I'd be happy if you take the
time for review.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/krichter722/storm retry-manager-config
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/2598.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2598
----
commit a1f66f80cbe72fb144d0ee624e90fa65ca04884d
Author: Karl-Philipp Richter <krichter722@...>
Date: 2018-03-19T04:48:08Z
storm-kafka: added ExponentialBackoffMsgRetryManagerSpoutConfig in order to
avoid duplicate and unused properties
----
---
