Github user Parth-Brahmbhatt commented on the pull request:
https://github.com/apache/storm/pull/387#issuecomment-72349915
Thanks for the suggestions, I have tried to address all the concerns below.
* I can create multiple jiras and separate the refactoring in two parts as
you suggested.
* I did not remove PartitionCoordinator, just moved it to a package called
partition where I moved all the partition management stuff.
* I removed IBrokerReader because I felt the recommended way to get the
kafka broker host is the only way we should support, like a final method that
can not be overridden. The users of this connector should not care how does the
connector get internal kafka information like a topics' broker->partition
mapping. If others are of the opinion that we should make this extensible I can
keep the interface around I personally vote against doing this.
* I have worked on few kafka connector issues and everytime I read the code
the marker interfaces and alternate configs were actually the things that made
reading/understanding the code harder. I could pass an instance of
tridentConfig to non trident spout and nothing would work and no compile time
errors/warnings will be issued. In my opinion we should not support 3 ways to
get internal kafka details, this does not just make the code harder to read
but even the documentation to use the connector gets confusing with so many
different ways to achieve something that should be internal to the connector
code.
---
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 [email protected] or file a JIRA ticket
with INFRA.
---