Re: [PR] [ZEPPELIN-6406] Remove deprecated Flink 1.15/1.16/1.17 shims and add Flink 1.19/1.20 support [zeppelin]

["jongyoul (via GitHub)" <gi...@apache.org>]

jongyoul commented on code in PR #5205:
URL: https://github.com/apache/zeppelin/pull/5205#discussion_r3171450706


##########
flink/flink-scala-2.12/src/main/scala/org/apache/zeppelin/flink/FlinkScalaInterpreter.scala:
##########
@@ -423,6 +423,26 @@ abstract class FlinkScalaInterpreter(val properties: Properties,
     setAsContext()
   }
 
+  private def bindWithRetry(name: String, tpe: String, value: AnyRef, modifiers: List[String]): Unit = {
+    // Workaround for Scala reflection issue with ImplicitExpressionConversions in Flink 1.19+.
+    // First bind attempt may fail due to unpickling error, but subsequent attempts succeed
+    // because the Scala reflection cache resolves the error state.

Review Comment:
   @pan3793 Got pulled into other work.
   
   On `bindWithRetry`: no public reference. AI-drafted, then I traced through the actual stacktrace below and adjusted from there. Does this look reasonable?
   
   ```
   WARN [...] FlinkScalaInterpreter.scala[bindWithRetry]:438 - Retrying bind for btenv due to Scala reflection issue java.lang.RuntimeException: error reading Scala signature of $line13.$read: error reading Scala signature of org.apache.flink.table.api.ImplicitExpressionConversions: unsafe symbol x$2 (child of value <local api>) in runtime reflection universe
       at scala.reflect.internal.pickling.UnPickler.unpickle(UnPickler.scala:51)
       at scala.reflect.runtime.JavaMirrors$JavaMirror.unpickleClass(JavaMirrors.scala:660)
       at scala.reflect.runtime.SymbolLoaders$TopClassCompleter.$anonfun$complete$2(SymbolLoaders.scala:37)
       at scala.reflect.internal.SymbolTable.slowButSafeEnteringPhaseNotLaterThan(SymbolTable.scala:333)
       at scala.reflect.runtime.SymbolLoaders$TopClassCompleter.complete(SymbolLoaders.scala:34)
       at scala.reflect.runtime.SymbolLoaders$TopClassCompleter.load(SymbolLoaders.scala:42)
       at scala.reflect.internal.Symbols$Symbol.exists(Symbols.scala:1104)
       at scala.reflect.runtime.SynchronizedSymbols$SynchronizedSymbol.exists(SynchronizedSymbols.scala:149)
       at scala.reflect.internal.Importers$StandardImporter.recreateOrRelink$1(Importers.scala:175)
       at scala.reflect.internal.Importers$StandardImporter.importSymbol(Importers.scala:222)
       at scala.reflect.internal.Importers$StandardImporter.importSymbol(Importers.scala:41)
       at scala.tools.nsc.interpreter.IMain.importFromRu(IMain.scala:1051)
       at scala.tools.nsc.interpreter.IMain.getModuleIfDefined(IMain.scala:162)
       at scala.tools.nsc.interpreter.IMain.readRootPath(IMain.scala:297)
       at scala.tools.nsc.interpreter.IMain$ReadEvalPrint.resolvePathToSymbol(IMain.scala:779)
       at scala.tools.nsc.interpreter.IMain$Request.resultSymbol$lzycompute(IMain.scala:994)
       at scala.tools.nsc.interpreter.IMain$Request.resultSymbol(IMain.scala:994)
       at scala.tools.nsc.interpreter.IMain$Request.typeOf$lzycompute(IMain.scala:1002)
       at scala.tools.nsc.interpreter.IMain$Request.typeOf(IMain.scala:1007)
       at scala.tools.nsc.interpreter.IMain$Request.compile$lzycompute(IMain.scala:974)
       at scala.tools.nsc.interpreter.IMain$Request.compile(IMain.scala:967)
       at scala.tools.nsc.interpreter.IMain.compile(IMain.scala:612)
       at scala.tools.nsc.interpreter.IMain.interpret(IMain.scala:592)
       at scala.tools.nsc.interpreter.IMain.interpret(IMain.scala:564)
       at scala.tools.nsc.interpreter.IMain.bind(IMain.scala:650)
       at org.apache.zeppelin.flink.FlinkScalaInterpreter.$anonfun$bindWithRetry$2(FlinkScalaInterpreter.scala:433)
       at org.apache.zeppelin.flink.FlinkScalaInterpreter.bindWithRetry(FlinkScalaInterpreter.scala:431)
       at org.apache.zeppelin.flink.FlinkScalaInterpreter.createTableEnvs(FlinkScalaInterpreter.scala:462)
       at org.apache.zeppelin.flink.FlinkScalaInterpreter.open(FlinkScalaInterpreter.scala:124)
       at org.apache.zeppelin.flink.FlinkInterpreter.open(FlinkInterpreter.java:71)
       at org.apache.zeppelin.flink.FlinkInterpreterTest.setUp(FlinkInterpreterTest.java:66)
   ```
   
   On Java 17: fully agree. I'll share it on the mailing list and start the work. A few interpreters will be affected, but they're on older versions anyway — let's share it first and gather opinions.



-- 
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: reviews-unsubscribe@zeppelin.apache.org

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