Github user wurstmeister commented on the pull request:
https://github.com/apache/storm/pull/387#issuecomment-72336141
Hi
thanks for starting this refactoring, i've had this on my list of things to
do but unfortunately have not had the time to do it.
A few suggestions:
It would be nice to break up this change (maybe we can even break up the
jira):
- ITuple refactoring & renaming of packages
- addition of API based PartitionCoordinator
This makes it easier to review.
Also do we really have to make this a breaking change? Could we not just
have another implementation of ```IBrokerReader``` and
```PartitionCoordinator```, which are fairly clean interfaces. Iâve
definitely found the static partition configuration helpful when debugging.
The use of the marker interface is fairly localised
(```KafkaUtils.makeBrokerReader``` and ```KafkaSpout.open```) so I don't think
it's too big a problem.
If we want to get away from the ```BrokerHosts``` marker interface we could
add something more explicit to the ```KafkaConfig``` (e.g.
```config.partitionSource()``` which could return STATIC/ZK/KAFKA) - I think
this is more a question of style rather than complexity.
Why did you remove the configuration for the batch coordinator in trident?
Iâve found that quite useful when debugging. I also think that mixing trident
and non trident configuration can be a bit confusing, some parameters, e.g.
```stateUpdateIntervalMs``` do not have a meaning in trident.
Thanks again!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
