You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@toree.apache.org by lr...@apache.org on 2018/01/12 07:49:45 UTC

incubator-toree git commit: [TOREE-456] Support for --alternate-sigint command-line option

Repository: incubator-toree
Updated Branches:
  refs/heads/master a9c295cc6 -> 7a09462ea


[TOREE-456] Support for --alternate-sigint command-line option

Added support for --alternate-sigint command-line option and
removed the usage of TOREE_ALTERNATE_SIGINT environment variable.
Changed the signature of HookInitialization.initializeHooks() to
pass in the Config. Added unit tests for the new command-line.
option.

Closes #150


Project: http://git-wip-us.apache.org/repos/asf/incubator-toree/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-toree/commit/7a09462e
Tree: http://git-wip-us.apache.org/repos/asf/incubator-toree/tree/7a09462e
Diff: http://git-wip-us.apache.org/repos/asf/incubator-toree/diff/7a09462e

Branch: refs/heads/master
Commit: 7a09462ea6d3f800ecd4b932f5f42f180c501b87
Parents: a9c295c
Author: Sanjay Saxena <sa...@gmail.com>
Authored: Thu Jan 11 16:05:49 2018 -0800
Committer: Luciano Resende <lr...@apache.org>
Committed: Thu Jan 11 23:49:03 2018 -0800

----------------------------------------------------------------------
 .../apache/toree/boot/CommandLineOptions.scala  |  7 +++++
 .../org/apache/toree/boot/KernelBootstrap.scala |  1 +
 .../toree/boot/layer/HookInitialization.scala   | 20 +++++++++-----
 .../toree/boot/CommandLineOptionsSpec.scala     | 29 ++++++++++++++++++++
 4 files changed, 50 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-toree/blob/7a09462e/kernel/src/main/scala/org/apache/toree/boot/CommandLineOptions.scala
----------------------------------------------------------------------
diff --git a/kernel/src/main/scala/org/apache/toree/boot/CommandLineOptions.scala b/kernel/src/main/scala/org/apache/toree/boot/CommandLineOptions.scala
index 4fc021f..13325a8 100644
--- a/kernel/src/main/scala/org/apache/toree/boot/CommandLineOptions.scala
+++ b/kernel/src/main/scala/org/apache/toree/boot/CommandLineOptions.scala
@@ -105,6 +105,12 @@ class CommandLineOptions(args: Seq[String]) {
       "The default value is 100 milliseconds."
   ).withRequiredArg().ofType(classOf[Long])
 
+  private val _alternate_sigint = parser.accepts(
+    "alternate-sigint",
+    "Specifies the signal to use instead of SIGINT for interrupting a long-running cell. " +
+      "The value is a string and does not include the SIG prefix. Use of USR2 is recommended."
+  ).withRequiredArg().ofType(classOf[String])
+
   private val options = parser.parse(args.map(_.trim): _*)
 
   /*
@@ -155,6 +161,7 @@ class CommandLineOptions(args: Seq[String]) {
         .flatMap(list => if (list.isEmpty) None else Some(list)),
       "max_interpreter_threads" -> get(_max_interpreter_threads),
       "spark_context_intialization_timeout" -> get(_spark_context_intialization_timeout),
+      "alternate_sigint" -> get(_alternate_sigint),
       "jar_dir" -> get(_jar_dir),
       "default_interpreter" -> get(_default_interpreter),
       "nosparkcontext" -> (if (has(_nosparkcontext)) Some(true) else Some(false)),

http://git-wip-us.apache.org/repos/asf/incubator-toree/blob/7a09462e/kernel/src/main/scala/org/apache/toree/boot/KernelBootstrap.scala
----------------------------------------------------------------------
diff --git a/kernel/src/main/scala/org/apache/toree/boot/KernelBootstrap.scala b/kernel/src/main/scala/org/apache/toree/boot/KernelBootstrap.scala
index cea0080..b4ccbc0 100644
--- a/kernel/src/main/scala/org/apache/toree/boot/KernelBootstrap.scala
+++ b/kernel/src/main/scala/org/apache/toree/boot/KernelBootstrap.scala
@@ -120,6 +120,7 @@ class KernelBootstrap(config: Config) extends LogLike {
 
     // Initialize our non-shutdown hooks that handle various JVM events
     initializeHooks(
+      config = config,
       interpreter = interpreter
     )
 

http://git-wip-us.apache.org/repos/asf/incubator-toree/blob/7a09462e/kernel/src/main/scala/org/apache/toree/boot/layer/HookInitialization.scala
----------------------------------------------------------------------
diff --git a/kernel/src/main/scala/org/apache/toree/boot/layer/HookInitialization.scala b/kernel/src/main/scala/org/apache/toree/boot/layer/HookInitialization.scala
index bf6cd0f..c47eb92 100644
--- a/kernel/src/main/scala/org/apache/toree/boot/layer/HookInitialization.scala
+++ b/kernel/src/main/scala/org/apache/toree/boot/layer/HookInitialization.scala
@@ -21,6 +21,8 @@ import org.apache.toree.boot.KernelBootstrap
 import org.apache.toree.interpreter.Interpreter
 import org.apache.toree.utils.LogLike
 
+import com.typesafe.config.Config
+
 /**
  * Represents the hook (interrupt/shutdown) initialization. All JVM-related
  * hooks should be constructed here.
@@ -29,9 +31,10 @@ trait HookInitialization {
   /**
    * Initializes and registers all hooks except shutdown.
    *
+   * @param config The main config
    * @param interpreter The main interpreter
    */
-  def initializeHooks(interpreter: Interpreter): Unit
+  def initializeHooks(config: Config, interpreter: Interpreter): Unit
 
   /**
    * Initializes the shutdown hook.
@@ -48,10 +51,11 @@ trait StandardHookInitialization extends HookInitialization {
   /**
    * Initializes and registers all hooks.
    *
+   * @param config The main config
    * @param interpreter The main interpreter
    */
-  def initializeHooks(interpreter: Interpreter): Unit = {
-    registerInterruptHook(interpreter)
+  def initializeHooks(config: Config, interpreter: Interpreter): Unit = {
+    registerInterruptHook(config, interpreter)
   }
 
   /**
@@ -61,7 +65,7 @@ trait StandardHookInitialization extends HookInitialization {
     registerShutdownHook()
   }
 
-  private def registerInterruptHook(interpreter: Interpreter): Unit = {
+  private def registerInterruptHook(config: Config, interpreter: Interpreter): Unit = {
     val self = this
 
     import sun.misc.{Signal, SignalHandler}
@@ -94,8 +98,10 @@ trait StandardHookInitialization extends HookInitialization {
     // cell operations - since SIGINT doesn't propagate in those cases.
     // Like INT above except we don't need to deal with shutdown in
     // repeated situations.
-    val altSigint = System.getenv("TOREE_ALTERNATE_SIGINT")
-    if (altSigint != null) {
+    val altSigintOption = "alternate_sigint"
+    if (config.hasPath(altSigintOption)) {
+      val altSigint = config.getString(altSigintOption)
+
       try {
         Signal.handle(new Signal(altSigint), new SignalHandler() {
 
@@ -109,7 +115,7 @@ trait StandardHookInitialization extends HookInitialization {
         })
       } catch {
         case e:Exception => logger.warn("Error occurred establishing alternate signal handler " +
-          "(TOREE_ALTERNATE_SIGINT = " + altSigint + ").  Error: " + e.getMessage )
+          "(--alternate-sigint = " + altSigint + ").  Error: " + e.getMessage )
       }
     }
   }

http://git-wip-us.apache.org/repos/asf/incubator-toree/blob/7a09462e/kernel/src/test/scala/org/apache/toree/boot/CommandLineOptionsSpec.scala
----------------------------------------------------------------------
diff --git a/kernel/src/test/scala/org/apache/toree/boot/CommandLineOptionsSpec.scala b/kernel/src/test/scala/org/apache/toree/boot/CommandLineOptionsSpec.scala
index d79014d..470b091 100644
--- a/kernel/src/test/scala/org/apache/toree/boot/CommandLineOptionsSpec.scala
+++ b/kernel/src/test/scala/org/apache/toree/boot/CommandLineOptionsSpec.scala
@@ -355,5 +355,34 @@ class CommandLineOptionsSpec extends FunSpec with Matchers {
         }
       }
     }
+
+    describe("when dealing with --alternate-sigint") {
+      val key = "alternate_sigint"
+
+      it("when option is not specified, Config.hasPath method must return false") {
+        val options = new CommandLineOptions(Nil)
+        val config: Config = options.toConfig
+        config.hasPath(key) should be(false)
+      }
+
+      it("when option is specified, the value must be returned") {
+        val options = new CommandLineOptions(List(
+          "--alternate-sigint", "foo"
+        ))
+        val config: Config = options.toConfig
+        config.getString(key) should be("foo")
+      }
+
+      it("when a value is not specified, an exception must be thrown") {
+        intercept[OptionException] {
+          val options = new CommandLineOptions(List(
+            "--alternate-sigint"
+          ))
+          val config: Config = options.toConfig
+        }
+      }
+
+    }
+
   }
 }