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.
---

Reply via email to