rionmonster commented on pull request #15175: URL: https://github.com/apache/flink/pull/15175#issuecomment-797867198
@zentol I decided to make another, different pass at this guy since the last PR seemed to be a bit all over the place. I migrated almost all of the logic to simplify the constructors as much as possible, but I'm sure some additional eyes and feedback would be appreciated. The only thing that really stuck out was the use of the `MetricConfig` that was used under the hood, however the factory methods (e.g. `createMetricReporter(...)`) accept `Properties`, so I added an explicit cast since `MetricConfig` was just a superset of `Properties`. Additionally, I updated 1-2 of the available unit tests to call the static method that was added to one of the factories and adjusted another that assumed an empty constructor for the `PrometheusReporter` which explicitly now requires the `port` and `httpServer` respectively. Perhaps we need another empty constructor as well? Anyways - it's late on this end, but hopefully this is a step in a better direction. Thanks much, Rion ---------------------------------------------------------------- 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
