Github user merrimanr commented on the issue:
https://github.com/apache/metron/pull/717
I tested this in full dev and everything worked as advertised. The only
major issue I see is that the num workers property is missing from
SensorParserConfig. I feel like this is one of the most important settings for
a topology. You could set it with "topology.workers" in the "stormConfig"
property but it's not obvious and I think this setting should be a first class
citizen like the others.
For me, the use of the ValueSupplier functional interface made it more
difficult to understand when reviewing. The code seemed more verbose and
complex and I wasn't able to work out the benefit of using it in this case.
It's is a very minor style issue and is just my (probably worthless) opinion.
I would not hold up the PR over something like this, just some honest feedback.
Awesome work. If we can promote num workers to a getter method in
SensorParserConfig then I'm confident giving a +1.
---
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.
---
