You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@storm.apache.org by krichter722 <gi...@git.apache.org> on 2018/03/19 06:56:15 UTC

[GitHub] storm pull request #2598: storm-kafka: added ExponentialBackoffMsgRetryManag...

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 <kr...@...>
Date:   2018-03-19T04:48:08Z

    storm-kafka: added ExponentialBackoffMsgRetryManagerSpoutConfig in order to avoid duplicate and unused properties

----


---