zentol commented on code in PR #21387:
URL: https://github.com/apache/flink/pull/21387#discussion_r1032178749
##########
flink-runtime/src/main/java/org/apache/flink/runtime/metrics/ReporterSetup.java:
##########
@@ -464,6 +466,21 @@ private static Optional<MetricReporter> loadViaReflection(
alternativeFactoryClassName, reporterName, reporterConfig,
reporterFactories);
}
- return Optional.of((MetricReporter) reporterClass.newInstance());
+ try {
+ return Optional.of((MetricReporter) reporterClass.newInstance());
+ } catch (InstantiationException e) {
+ if (ExceptionUtils.findThrowable(e,
NoSuchMethodException.class).isPresent()) {
Review Comment:
This may not be exhaustive enough. Reporters that are exclusively
factory-based may not even be public anymore.
##########
flink-runtime/src/main/java/org/apache/flink/runtime/metrics/ReporterSetup.java:
##########
@@ -464,6 +466,21 @@ private static Optional<MetricReporter> loadViaReflection(
alternativeFactoryClassName, reporterName, reporterConfig,
reporterFactories);
}
- return Optional.of((MetricReporter) reporterClass.newInstance());
+ try {
+ return Optional.of((MetricReporter) reporterClass.newInstance());
+ } catch (InstantiationException e) {
+ if (ExceptionUtils.findThrowable(e,
NoSuchMethodException.class).isPresent()) {
+ throw new IllegalConfigurationException(
+ String.format(
+ "%s does not implement a default constructor
but is explicitly configured. "
+ + "Reflection-based instantiation of
reporter instances is deprecated "
+ + "and does not necessarily work.
Instead, factory-based instantiation "
Review Comment:
Maybe adjust the existing message in L#383 instead. Add that "reporters may
no longer support it" and we're good to go.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
