Merge pull request #572 from hex-inc/gt/reintroduce-539

Re-introduce #539, fix abort to not abort non-execute requests

Author: blink1073

ipykernel/kernelbase.py

@@ -40,7 +40,6 @@
 
 CONTROL_PRIORITY = 1
 SHELL_PRIORITY = 10
-ABORT_PRIORITY = 20
 
 
 class Kernel(SingletonConfigurable):
@@ -182,10 +181,6 @@ def dispatch_control(self, msg):
         # Set the parent message for side effects.
         self.set_parent(idents, msg)
         self._publish_status('busy')
-        if self._aborting:
-            self._send_abort_reply(self.control_stream, msg, idents)
-            self._publish_status('idle')
-            return
 
         header = msg['header']
         msg_type = header['msg_type']
@@ -233,16 +228,17 @@ def dispatch_shell(self, stream, msg):
         self.set_parent(idents, msg)
         self._publish_status('busy')
 
-        if self._aborting:
+        msg_type = msg['header']['msg_type']
+
+        # Only abort execute requests
+        if self._aborting and msg_type == 'execute_request':
             self._send_abort_reply(stream, msg, idents)
             self._publish_status('idle')
             # flush to ensure reply is sent before
             # handling the next request
             stream.flush(zmq.POLLOUT)
             return
 
-        msg_type = msg['header']['msg_type']
-
         # Print some info about this message and leave a '--->' marker, so it's
         # easier to trace visually the message chain when debugging.  Each
         # handler prints its message at the end.
@@ -792,16 +788,11 @@ def _abort_queues(self):
             stream.flush()
         self._aborting = True
 
-        self.schedule_dispatch(
-            ABORT_PRIORITY,
-            self._dispatch_abort,
-        )
+        def stop_aborting(f):
+            self.log.info("Finishing abort")
+            self._aborting = False
 
-    @gen.coroutine
-    def _dispatch_abort(self):
-        self.log.info("Finishing abort")
-        yield gen.sleep(self.stop_on_error_timeout)
-        self._aborting = False
+        self.io_loop.add_future(gen.sleep(self.stop_on_error_timeout), stop_aborting)
 
     def _send_abort_reply(self, stream, msg, idents):
         """Send a reply to an aborted request"""

ipykernel/tests/test_message_spec.py

@@ -114,6 +114,8 @@ def check(self, d):
             ExecuteReplyOkay().check(d)
         elif d['status'] == 'error':
             ExecuteReplyError().check(d)
+        elif d['status'] == 'aborted':
+            ExecuteReplyAborted().check(d)
 
 
 class ExecuteReplyOkay(Reply):
@@ -122,11 +124,16 @@ class ExecuteReplyOkay(Reply):
 
 
 class ExecuteReplyError(Reply):
+    status = Enum(('error',))
     ename = Unicode()
     evalue = Unicode()
     traceback = List(Unicode())
 
 
+class ExecuteReplyAborted(Reply):
+    status = Enum(('aborted',))
+
+
 class InspectReply(Reply, MimeBundle):
     found = Bool()
 
@@ -348,6 +355,30 @@ def test_execute_stop_on_error():
     assert reply['content']['status'] == 'ok'
 
 
+def test_non_execute_stop_on_error():
+    """test that non-execute_request's are not aborted after an error"""
+    flush_channels()
+
+    fail = '\n'.join([
+        # sleep to ensure subsequent message is waiting in the queue to be aborted
+        'import time',
+        'time.sleep(0.5)',
+        'raise ValueError',
+    ])
+    KC.execute(code=fail)
+    KC.kernel_info()
+    KC.comm_info()
+    KC.inspect(code="print")
+    reply = KC.get_shell_msg(timeout=TIMEOUT) # execute
+    assert reply['content']['status'] == 'error'
+    reply = KC.get_shell_msg(timeout=TIMEOUT) # kernel_info
+    assert reply['content']['status'] == 'ok'
+    reply = KC.get_shell_msg(timeout=TIMEOUT) # comm_info
+    assert reply['content']['status'] == 'ok'
+    reply = KC.get_shell_msg(timeout=TIMEOUT) # inspect
+    assert reply['content']['status'] == 'ok'
+
+
 def test_user_expressions():
     flush_channels()