You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/11/11 10:44:46 UTC

[GitHub] [flink] zentol opened a new pull request, #21296: [FLINK-24235][metrics] Only support reporters via factories

zentol opened a new pull request, #21296:
URL: https://github.com/apache/flink/pull/21296

   Following the deprecation in 1.16 this PR removes the reflection-based code path for instantiating reporters.


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] zentol commented on a diff in pull request #21296: [FLINK-24235][metrics] Only support reporters via factories

Posted by GitBox <gi...@apache.org>.
zentol commented on code in PR #21296:
URL: https://github.com/apache/flink/pull/21296#discussion_r1020105044


##########
flink-metrics/flink-metrics-core/src/main/java/org/apache/flink/metrics/reporter/InstantiateViaFactory.java:
##########
@@ -26,14 +26,7 @@
 import java.lang.annotation.Target;
 
 /**
- * Annotation for {@link MetricReporter MetricReporters} that support factories but want to maintain
- * backwards-compatibility with existing reflection-based configurations.
- *
- * <p>When an annotated reporter is configured to be used via reflection the given factory will be
- * used instead.
- *
- * <p>Attention: This annotation does not work if the reporter is loaded as a plugin. For these
- * cases, annotate the factory with {@link InterceptInstantiationViaReflection} instead.
+ * This annotation has no effect and is only kept for compatibility reasons.

Review Comment:
   We are keeping these around because reporters/factories that used them were already set up correctly to support factories (else there's no reason to use these annotations!).
   
   I didn't want to break these reporters, when really only the configuration should be updated.



-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] zentol commented on pull request #21296: [FLINK-24235][metrics] Only support reporters via factories

Posted by GitBox <gi...@apache.org>.
zentol commented on PR #21296:
URL: https://github.com/apache/flink/pull/21296#issuecomment-1313544335

   @flinkbot run azure


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] flinkbot commented on pull request #21296: [FLINK-24235][metrics] Only support reporters via factories

Posted by GitBox <gi...@apache.org>.
flinkbot commented on PR #21296:
URL: https://github.com/apache/flink/pull/21296#issuecomment-1311538985

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "97c90cf38fdad8e9cf4763fb84156d874658e465",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "97c90cf38fdad8e9cf4763fb84156d874658e465",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 97c90cf38fdad8e9cf4763fb84156d874658e465 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run azure` re-run the last Azure build
   </details>


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] zentol merged pull request #21296: [FLINK-24235][metrics] Only support reporters via factories

Posted by GitBox <gi...@apache.org>.
zentol merged PR #21296:
URL: https://github.com/apache/flink/pull/21296


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] zentol commented on a diff in pull request #21296: [FLINK-24235][metrics] Only support reporters via factories

Posted by GitBox <gi...@apache.org>.
zentol commented on code in PR #21296:
URL: https://github.com/apache/flink/pull/21296#discussion_r1020105044


##########
flink-metrics/flink-metrics-core/src/main/java/org/apache/flink/metrics/reporter/InstantiateViaFactory.java:
##########
@@ -26,14 +26,7 @@
 import java.lang.annotation.Target;
 
 /**
- * Annotation for {@link MetricReporter MetricReporters} that support factories but want to maintain
- * backwards-compatibility with existing reflection-based configurations.
- *
- * <p>When an annotated reporter is configured to be used via reflection the given factory will be
- * used instead.
- *
- * <p>Attention: This annotation does not work if the reporter is loaded as a plugin. For these
- * cases, annotate the factory with {@link InterceptInstantiationViaReflection} instead.
+ * This annotation has no effect and is only kept for compatibility reasons.

Review Comment:
   We are keeping these around because reporters/factories that used them were already set up correctly to at least support factories (else there's no reason to use these annotations!).
   
   I didn't want to break these reporters, when really only the configuration should be updated.



-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org