From 5fea47ffe053278ef0a1a10603f3e0e314188cc9 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Thu, 6 Sep 2018 12:11:02 +0200 Subject: [PATCH 1/3] Move pre-initialization code to ScalaKernel Better not to call sys.exit from ScalaInterpreter, which may be used as a library in other contexts --- .../main/scala/almond/ScalaInterpreter.scala | 22 ++----------------- .../src/main/scala/almond/ScalaKernel.scala | 20 +++++++++++++++++ 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/modules/scala/scala-interpreter/src/main/scala/almond/ScalaInterpreter.scala b/modules/scala/scala-interpreter/src/main/scala/almond/ScalaInterpreter.scala index 7fe8f77d6..faea3eaea 100644 --- a/modules/scala/scala-interpreter/src/main/scala/almond/ScalaInterpreter.scala +++ b/modules/scala/scala-interpreter/src/main/scala/almond/ScalaInterpreter.scala @@ -326,29 +326,11 @@ final class ScalaInterpreter( ammInterp0 } catch { case t: Throwable => - log.error(s"Caught exception while initializing interpreter, exiting", t) - sys.exit(1) + log.error(s"Caught exception while initializing interpreter", t) + throw t } } - // Actually init interpreter in background - - private val initThread = new Thread("interpreter-init") { - setDaemon(true) - override def run() = - try { - log.info("Initializing interpreter (background)") - ammInterp - log.info("Initialized interpreter (background)") - } - catch { - case t: Throwable => - log.error(s"Caught exception while initializing interpreter", t) - } - } - - initThread.start() - private var interruptedStackTraceOpt = Option.empty[Array[StackTraceElement]] private var currentThreadOpt = Option.empty[Thread] diff --git a/modules/scala/scala-kernel/src/main/scala/almond/ScalaKernel.scala b/modules/scala/scala-kernel/src/main/scala/almond/ScalaKernel.scala index 7c0ca04d7..c2cfffedf 100644 --- a/modules/scala/scala-kernel/src/main/scala/almond/ScalaKernel.scala +++ b/modules/scala/scala-kernel/src/main/scala/almond/ScalaKernel.scala @@ -133,6 +133,26 @@ object ScalaKernel extends CaseApp[Options] { ) log.info("Created interpreter") + + // Actually init Ammonite interpreter in background + + val initThread = new Thread("interpreter-init") { + setDaemon(true) + override def run() = + try { + log.info("Initializing interpreter (background)") + interpreter.ammInterp + log.info("Initialized interpreter (background)") + } catch { + case t: Throwable => + log.error(s"Caught exception while initializing interpreter, exiting", t) + sys.exit(1) + } + } + + initThread.start() + + log.info("Running kernel") Kernel.create(interpreter, interpreterEc, kernelThreads, logCtx) .flatMap(_.runOnConnectionFile(connectionFile, "scala", zeromqThreads)) From 7bbedad6e3e2f5edc6e03ab3cdbe09ecc988f098 Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Thu, 6 Sep 2018 12:11:02 +0200 Subject: [PATCH 2/3] Tweak predef handling, test it --- .../main/scala/almond/ScalaInterpreter.scala | 53 +++++++------- .../scala/almond/ScalaInterpreterTests.scala | 72 +++++++++++++++++++ 2 files changed, 100 insertions(+), 25 deletions(-) diff --git a/modules/scala/scala-interpreter/src/main/scala/almond/ScalaInterpreter.scala b/modules/scala/scala-interpreter/src/main/scala/almond/ScalaInterpreter.scala index faea3eaea..3672f613e 100644 --- a/modules/scala/scala-interpreter/src/main/scala/almond/ScalaInterpreter.scala +++ b/modules/scala/scala-interpreter/src/main/scala/almond/ScalaInterpreter.scala @@ -146,6 +146,22 @@ final class ScalaInterpreter( } + def runPredef(interp: ammonite.interp.Interpreter): Unit = { + val stmts = Parsers.split(predef).get.get.value + val predefRes = interp.processLine(predef, stmts, 9999999, silent = false, () => ()) + Repl.handleOutput(interp, predefRes) + predefRes match { + case Res.Success(_) => + case Res.Failure(msg) => + throw new ScalaInterpreter.PredefException(msg, None) + case Res.Exception(t, msg) => + throw new ScalaInterpreter.PredefException(msg, Some(t)) + case Res.Skip => + case Res.Exit(v) => + log.warn(s"Ignoring exit request from predef (exit value: $v)") + } + } + lazy val ammInterp: ammonite.interp.Interpreter = { val replApi: ReplApiImpl = @@ -281,31 +297,7 @@ final class ScalaInterpreter( log.info(s"Warming up interpreter (predef: $predef)") - val code = - if (predef.isEmpty) - // from ammonite.repl.Repl.warmup() - """val array = Seq.tabulate(10)(_*2).toArray.max""" - else - predef - - def handlePredef(): Unit = { - val stmts = Parsers.split(code).get.get.value - val predefRes = ammInterp0.processLine(code, stmts, 9999999, silent = false, () => ()) - Repl.handleOutput(ammInterp0, predefRes) - predefRes match { - case Res.Success(_) => - case Res.Failure(msg) => - log.error(s"Error while running predef: $msg") - sys.exit(1) - case Res.Exception(t, msg) => - log.error(s"Caught exception while running predef ($msg)", t) - case Res.Skip => - case Res.Exit(v) => - log.warn(s"Ignoring exit request from predef (exit value: $v)") - } - } - - handlePredef() + runPredef(ammInterp0) log.info("Ammonite interpreter ok") @@ -533,6 +525,17 @@ final class ScalaInterpreter( object ScalaInterpreter { + final class PredefException( + msg: String, + causeOpt: Option[Throwable] + ) extends Exception(msg, causeOpt.orNull) { + def describe: String = + if (causeOpt.isEmpty) + s"Error while running predef: $msg" + else + s"Caught exception while running predef: $msg" + } + // these come from Ammonite // exception display was tweaked a bit (too much red for notebooks else) diff --git a/modules/scala/scala-interpreter/src/test/scala/almond/ScalaInterpreterTests.scala b/modules/scala/scala-interpreter/src/test/scala/almond/ScalaInterpreterTests.scala index ac1332f37..6fbfca7d5 100644 --- a/modules/scala/scala-interpreter/src/test/scala/almond/ScalaInterpreterTests.scala +++ b/modules/scala/scala-interpreter/src/test/scala/almond/ScalaInterpreterTests.scala @@ -66,6 +66,78 @@ object ScalaInterpreterTests extends TestSuite { } + "predef" - { + + "simple" - { + val predef = "val n = 2" + val interp = new ScalaInterpreter( + initialColors = Colors.BlackWhite, + predef = predef + ) + + val res = interp.execute("val m = 2 * n") + val expectedRes = ExecuteResult.Success(DisplayData.text("m: Int = 4")) + assert(res == expectedRes) + } + + "no variable name" - { + val predef = + """println("foo") // automatically generated: val res… = println("foo") + |val n = 2 + """.stripMargin + val interp = new ScalaInterpreter( + initialColors = Colors.BlackWhite, + predef = predef + ) + + val res = interp.execute("val m = 2 * n") + val expectedRes = ExecuteResult.Success(DisplayData.text("m: Int = 4")) + assert(res == expectedRes) + } + + "compilation error" - { + val predef = "val n = 2z" + val interp = new ScalaInterpreter( + initialColors = Colors.BlackWhite, + predef = predef + ) + + val res = + try { + interp.execute("val m = 2 * n") + false + } catch { + case e: ScalaInterpreter.PredefException => + assert(e.getCause == null) + true + } + + assert(res) + } + + "exception" - { + val predef = """val n: Int = sys.error("foo")""" + val interp = new ScalaInterpreter( + initialColors = Colors.BlackWhite, + predef = predef + ) + + val res = + try { + interp.execute("val m = 2 * n") + false + } catch { + case e: ScalaInterpreter.PredefException => + val msgOpt = Option(e.getCause).flatMap(e0 => Option(e0.getMessage)) + assert(msgOpt.contains("foo")) + true + } + + assert(res) + } + + } + } } From 44c0272d2da0b67d7fe05e6bea7cf8606d894bbf Mon Sep 17 00:00:00 2001 From: Alexandre Archambault Date: Thu, 6 Sep 2018 12:11:03 +0200 Subject: [PATCH 3/3] Fix message metadata type Metadata values can have any shape, see https://jupyter-client.readthedocs.io/en/5.2.3/messaging.html#general-message-format --- build.sbt | 3 +- .../scala/almond/interpreter/Message.scala | 8 +- .../almond/interpreter/MessageTests.scala | 76 +++++++++++++++++++ 3 files changed, 82 insertions(+), 5 deletions(-) create mode 100644 modules/shared/interpreter/src/test/scala/almond/interpreter/MessageTests.scala diff --git a/build.sbt b/build.sbt index 08b1ad884..d41ec4c68 100644 --- a/build.sbt +++ b/build.sbt @@ -53,7 +53,8 @@ lazy val interpreter = project .underShared .dependsOn(`interpreter-api`, protocol) .settings( - shared + shared, + testSettings ) lazy val kernel = project diff --git a/modules/shared/interpreter/src/main/scala/almond/interpreter/Message.scala b/modules/shared/interpreter/src/main/scala/almond/interpreter/Message.scala index 9e35706e2..fbc1c9993 100644 --- a/modules/shared/interpreter/src/main/scala/almond/interpreter/Message.scala +++ b/modules/shared/interpreter/src/main/scala/almond/interpreter/Message.scala @@ -19,7 +19,7 @@ final case class Message[T]( content: T, parent_header: Option[Header] = None, idents: List[Seq[Byte]] = Nil, - metadata: Map[String, String] = Map.empty + metadata: Map[String, Json] = Map.empty ) { def messageType: MessageType[T] = @@ -39,7 +39,7 @@ final case class Message[T]( def publish[U]( messageType: MessageType[U], content: U, - metadata: Map[String, String] = Map.empty, + metadata: Map[String, Json] = Map.empty, ident: Option[String] = None ): Message[U] = Message( @@ -56,7 +56,7 @@ final case class Message[T]( def reply[U]( messageType: MessageType[U], content: U, - metadata: Map[String, String] = Map.empty + metadata: Map[String, Json] = Map.empty ): Message[U] = Message( replyHeader(messageType), @@ -118,7 +118,7 @@ object Message { def parse[T: DecodeJson](rawMessage: RawMessage): Either[String, Message[T]] = for { header <- rawMessage.header.decodeEither[Header].right - metaData <- rawMessage.metadata.decodeEither[Map[String, String]].right + metaData <- rawMessage.metadata.decodeEither[Map[String, Json]].right content <- rawMessage.content.decodeEither[T].right } yield { // FIXME Parent header is optional, but this unnecessarily traps errors diff --git a/modules/shared/interpreter/src/test/scala/almond/interpreter/MessageTests.scala b/modules/shared/interpreter/src/test/scala/almond/interpreter/MessageTests.scala new file mode 100644 index 000000000..8258ea4dc --- /dev/null +++ b/modules/shared/interpreter/src/test/scala/almond/interpreter/MessageTests.scala @@ -0,0 +1,76 @@ +package almond.interpreter + +import almond.channels.{Message => RawMessage} +import almond.protocol.Header +import argonaut.Json +import utest._ + +object MessageTests extends TestSuite { + + val tests = Tests { + "metadata" - { + "empty" - { + val rawMsg = RawMessage( + Nil, + """{"username":"","version":"5.2","session":"66fee418-b43a-42b2-bba9-cc91ffac014a","msg_id":"40fe2409-d5ad-4a5d-a71c-31411eeb2ea5","msg_type":"execute_request","date":"2018-09-06T08:27:35.616295Z"}""", + "{}", + "{}", + """{"silent":false,"store_history":true}""" + ) + + val res = Message.parse[Json](rawMsg) + val expectedRes = Right( + Message( + Header( + "40fe2409-d5ad-4a5d-a71c-31411eeb2ea5", + "", + "66fee418-b43a-42b2-bba9-cc91ffac014a", + "execute_request", + Some("5.2") + ), + Json.obj( + "silent" -> Json.jBool(false), + "store_history" -> Json.jBool(true) + ) + ) + ) + + assert(res == expectedRes) + } + + "jupyterlab-like" - { + val rawMsg = RawMessage( + Nil, + """{"username":"","version":"5.2","session":"66fee418-b43a-42b2-bba9-cc91ffac014a","msg_id":"40fe2409-d5ad-4a5d-a71c-31411eeb2ea5","msg_type":"execute_request","date":"2018-09-06T08:27:35.616295Z"}""", + "{}", + """{"deletedCells":[],"cellId":"7ff41107-c89a-4272-8ea2-d6433f918a6d"}""", + """{"silent":false,"store_history":true}""" + ) + + val res = Message.parse[Json](rawMsg) + val expectedRes = Right( + Message( + Header( + "40fe2409-d5ad-4a5d-a71c-31411eeb2ea5", + "", + "66fee418-b43a-42b2-bba9-cc91ffac014a", + "execute_request", + Some("5.2") + ), + Json.obj( + "silent" -> Json.jBool(false), + "store_history" -> Json.jBool(true) + ), + metadata = Map( + "deletedCells" -> Json.array(), + "cellId" -> Json.jString("7ff41107-c89a-4272-8ea2-d6433f918a6d") + ) + ) + ) + + assert(res == expectedRes) + } + } + } + +}