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 <sanjay.saxena...@gmail.com> Authored: Thu Jan 11 16:05:49 2018 -0800 Committer: Luciano Resende <lrese...@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 + } + } + + } + } }