mmiklavc commented on issue #1383: METRON-1788 Batch profiler pull profile information from zookeeper URL: https://github.com/apache/metron/pull/1383#issuecomment-488368420 This is looking really good @tigerquoll. I have a question about the implementation details in the test changes around TestZKServer.runWithZK. We generally try to keep our start/stop for beefier components to the @Before/AfterClass scope with intermediate cleanup at the individual test level so that we manage overall build times. It looks like this splits up some tests (to be clear, I'm very much in favor of taking something named `test` and splitting it into well named units for clarity) and does a full spin up and teardown of ZK for every individual test. That's probably not what we want. Can you talk to that a bit more? Other than this detail, I think this PR is in good shape and I would concur with @nickwallen 's +1 upon just a bit further explanation and/or tweaking of that test setup.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
