gjacoby126 commented on code in PR #9452:
URL: https://github.com/apache/iceberg/pull/9452#discussion_r1453839867


##########
flink/v1.16/flink/src/main/java/org/apache/iceberg/flink/util/FlinkPackage.java:
##########
@@ -19,15 +19,31 @@
 package org.apache.iceberg.flink.util;
 
 import org.apache.flink.streaming.api.datastream.DataStream;
+import 
org.apache.iceberg.relocated.com.google.common.annotations.VisibleForTesting;
 
 public class FlinkPackage {
-  /** Choose {@link DataStream} class because it is one of the core Flink API. 
*/
-  private static final String VERSION = 
DataStream.class.getPackage().getImplementationVersion();
+
+  public static final String FLINK_UNKNOWN_VERSION = "Flink-UNKNOWN";
 
   private FlinkPackage() {}
 
   /** Returns Flink version string like x.y.z */
   public static String version() {
-    return VERSION;
+    try {
+      String version = getVersionFromJar();
+      /* If we can't detect the exact implementation version from the jar 
(this can happen if the DataStream class
+       appears multiple times in the same classpath such as with shading), 
then the best we can do is say it's
+       unknown
+      */
+      return version != null ? version : FLINK_UNKNOWN_VERSION;

Review Comment:
   @stevenzwu - making VERSION `private static final String` makes it 
untestable, because you can't test the normal and default code paths in the 
same JVM. 
   
   @nastra @pvary - Using an AtomicReference would work, and if that's what you 
want I'll do it, but isn't that more complexity than this logic requires? 
`private static volatile String` works just fine, aside from the checkstyle 
issue mentioned above -- I need to either tweak the checkstyle settings, or 
give it a camelCase name. 



-- 
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-unsubscr...@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to