Github user nickwallen commented on the issue:

    https://github.com/apache/incubator-metron/pull/395
  
    As to @mattf-horton suggestion, I had looked at the possibility of not 
using the BaseWindowedBolt and implementing that myself.  But that seems like 
worse technical debt to me.  I'd be worried about how Storm evolves the 
functionality in future versions, especially since the functionality is 
relatively new in Storm.  I'm sure you have the same concerns, you're just 
trying to help find some compromise solution here.
    
    @cestella I see what you're going for, but don't see the logic in 
reimplementing what I already have working in `ZkConfigurationManager`.  What 
if I moved `ZkConfigurationManager` and the related classes to the 
`metron-profiler-common` project.  This would ensure that it could only be used 
exclusively by the Profiler.  Then as a follow-on PR, I will reach feature 
parity with ConfiguredBolt, move `ZkConfigurationManager` back to 
`metron-common`, update `ConfiguredBolt` to use `ZkConfigurationManager`, and 
all else that is needed there.
    
    I would suggest another approach as an alternative.  This satisfies the 
concerns for multiple implementations, and results in smaller, more concise PRs.
    1. Retract this PR
    2. Enhance ZkConfigurationManager to reach feature parity with 
ConfiguredBolt, update `ConfiguredBolt` to use `ZkConfigurationManager`, then 
submit that work as a separate PR. 
    3. Create a second PR for the new client-side Profiler functions as this 
has had its own lengthy discussion back-and-forth.
    4. Create a third PR for the event time processing additions in the 
Profiler.



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