Addressing PR comments

Author: Golf Player

nbclient/client.py

@@ -223,33 +223,43 @@ class NotebookClient(LoggingConfigurable):
 
     kernel_manager_class = Type(config=True, help='The kernel manager class to use.')
 
-    on_kernel_create = Any(
+    on_execution_start = Any(
         default_value=None,
         allow_none=True,
-        help="""A callable which executes when the kernel is created.""",
+        help=dedent("""
+        Called after the kernel manager and kernel client are setup, and cells 
+        are about to execute.
+        Called with kwargs `kernel_id`.
+        """),
     ).tag(config=True)
 
     on_cell_start = Any(
         default_value=None,
         allow_none=True,
-        help="""A callable which executes before a cell is executed.""",
+        help=dedent("""
+        A callable which executes before a cell is executed.
+        Called with kwargs `cell`, and `cell_index`.
+        """),
     ).tag(config=True)
 
     on_cell_complete = Any(
         default_value=None,
         allow_none=True,
-        help=dedent(
-            """
-            A callable which executes after a cell execution is complete. It is
-            called even when a cell results in a failure.
-            """
-        ),
+        help=dedent("""
+        A callable which executes after a cell execution is complete. It is
+        called even when a cell results in a failure.
+        Called with kwargs `cell`, and `cell_index`.
+        """),
     ).tag(config=True)
 
     on_cell_error = Any(
         default_value=None,
         allow_none=True,
-        help="""A callable which executes when a cell execution results in an error.""",
+        help=dedent("""
+        A callable which executes when a cell execution results in an error. 
+        This is executed even if errors are suppressed with `cell_allows_errors`.
+        Called with kwargs `cell`, and `cell_index`.
+        """),
     ).tag(config=True)
 
     @default('kernel_manager_class')
@@ -407,7 +417,6 @@ async def async_start_new_kernel_client(self, **kwargs):
 
         kernel_id = await ensure_async(self.km.start_kernel(extra_arguments=self.extra_arguments,
                                        **kwargs))
-        run_hook(self.on_kernel_create, kernel_id)
 
         # if self.km is not a KernelManager, it's probably a MultiKernelManager
         try:
@@ -431,6 +440,7 @@ async def async_start_new_kernel_client(self, **kwargs):
             await self._async_cleanup_kernel()
             raise
         self.kc.allow_stdin = False
+        run_hook(self.on_execution_start, kernel_id=kernel_id)
         return self.kc, kernel_id
 
     start_new_kernel_client = run_sync(async_start_new_kernel_client)
@@ -697,7 +707,7 @@ def _check_raise_for_error(self, cell, cell_index, exec_reply):
         )
 
         if (exec_reply is not None) and exec_reply['content']['status'] == 'error':
-            run_hook(self.on_cell_error, cell, cell_index)
+            run_hook(self.on_cell_error, cell=cell, cell_index=cell_index)
             if self.force_raise_errors or not cell_allows_errors:
                 raise CellExecutionError.from_cell_and_msg(cell, exec_reply['content'])
 
@@ -743,14 +753,15 @@ async def async_execute_cell(self, cell, cell_index, execution_count=None, store
             cell['metadata']['execution'] = {}
 
         self.log.debug("Executing cell:\n%s", cell.source)
-        run_hook(self.on_cell_start, cell, cell_index)
+        run_hook(self.on_cell_start, cell=cell, cell_index=cell_index)
         parent_msg_id = await ensure_async(
             self.kc.execute(
                 cell.source,
                 store_history=store_history,
                 stop_on_error=not self.allow_errors
             )
         )
+        run_hook(self.on_cell_complete, cell=cell, cell_index=cell_index)
         # We launched a code cell to execute
         self.code_cells_executed += 1
         exec_timeout = self._get_timeout(cell)

nbclient/util.py

@@ -6,6 +6,7 @@
 import asyncio
 import sys
 import inspect
+from functools import partial
 
 
 def check_ipython():
@@ -91,12 +92,13 @@ async def ensure_async(obj):
     return obj
 
 
-def run_hook(hook, *args):
+def run_hook(hook, **kwargs):
     if hook is None:
         return
     if inspect.iscoroutinefunction(hook):
-        future = hook(*args)
+        future = hook(**kwargs)
     else:
         loop = asyncio.get_event_loop()
-        future = loop.run_in_executor(None, hook, *args)
+        hook_with_kwargs = partial(hook, **kwargs)
+        future = loop.run_in_executor(None, hook_with_kwargs)
     asyncio.ensure_future(future)