yzeng1618 commented on code in PR #9576:
URL: https://github.com/apache/seatunnel/pull/9576#discussion_r2583925283


##########
seatunnel-translation/seatunnel-translation-flink/seatunnel-translation-flink-20/src/main/java/org/apache/seatunnel/translation/flink/metric/FlinkJobMetricsSummary.java:
##########


Review Comment:
   Thank you for the suggestion! The reason for implementing  
FlinkJobMetricsSummary separately in the flink-20 module was primarily due to 
the significant implementation differences in other metric classes (such as 
FlinkMetricContext, FlinkCounter, FlinkAccumulatorCounter) across different 
Flink versions. Following the principle of minimal impact, we wanted to avoid 
potential side effects on existing Flink 1.13/1.15 versions, hence the separate 
implementation approach.
   That said, you're absolutely right. Merging the  FlinkJobMetricsSummary 
enhancements into the common module would reduce code duplication and unify 
behavior across all Flink versions, which is indeed a better approach. I'll 
give it a try later.
   



-- 
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]

Reply via email to