Skip to content

Commit 33d5198

Browse files
kevin-batestoonijn
authored andcommitted
Apply changes per review
Add comments and rename self.super to self.pinned_superclass to clarify intent. Add run_sync() util method to clean up shutdown_all() invocation.
1 parent c927a3a commit 33d5198

File tree

3 files changed

+55
-19
lines changed

3 files changed

+55
-19
lines changed

notebook/notebookapp.py

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@
107107
from notebook._sysinfo import get_sys_info
108108

109109
from ._tz import utcnow, utcfromtimestamp
110-
from .utils import url_path_join, check_pid, url_escape, urljoin, pathname2url
110+
from .utils import url_path_join, check_pid, url_escape, urljoin, pathname2url, run_sync
111111

112112
# Check if we can use async kernel management
113113
try:
@@ -1801,11 +1801,7 @@ def cleanup_kernels(self):
18011801
n_kernels = len(self.kernel_manager.list_kernel_ids())
18021802
kernel_msg = trans.ngettext('Shutting down %d kernel', 'Shutting down %d kernels', n_kernels)
18031803
self.log.info(kernel_msg % n_kernels)
1804-
# If we're using async kernel management, we need to invoke the async method via the event loop.
1805-
if isinstance(self.kernel_manager, AsyncMappingKernelManager):
1806-
asyncio.get_event_loop().run_until_complete(self.kernel_manager.shutdown_all())
1807-
else:
1808-
self.kernel_manager.shutdown_all()
1804+
run_sync(self.kernel_manager.shutdown_all())
18091805

18101806
def notebook_info(self, kernel_count=True):
18111807
"Return the current working directory and the server url information"

notebook/services/kernels/kernelmanager.py

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,6 @@ def _default_kernel_buffers(self):
122122
last_kernel_activity = Instance(datetime,
123123
help="The last activity on any kernel, including shutting down a kernel")
124124

125-
def __init__(self, **kwargs):
126-
super().__init__(**kwargs)
127-
self.last_kernel_activity = utcnow()
128-
129125
allowed_message_types = List(trait=Unicode(), config=True,
130126
help="""White list of allowed kernel message types.
131127
When the list is empty, all message types are allowed.
@@ -137,8 +133,11 @@ def __init__(self, **kwargs):
137133
#-------------------------------------------------------------------------
138134

139135
def __init__(self, **kwargs):
140-
self.super = MultiKernelManager
141-
self.super.__init__(self, **kwargs)
136+
# Pin the superclass to better control the MRO. This is needed by
137+
# AsyncMappingKernelManager so that it can give priority to methods
138+
# on AsyncMultiKernelManager over this superclass.
139+
self.pinned_superclass = MultiKernelManager
140+
self.pinned_superclass.__init__(self, **kwargs)
142141
self.last_kernel_activity = utcnow()
143142

144143
def _handle_kernel_died(self, kernel_id):
@@ -173,7 +172,7 @@ async def start_kernel(self, kernel_id=None, path=None, **kwargs):
173172
if kernel_id is None:
174173
if path is not None:
175174
kwargs['cwd'] = self.cwd_for_path(path)
176-
kernel_id = await maybe_future(self.super.start_kernel(self, **kwargs))
175+
kernel_id = await maybe_future(self.pinned_superclass.start_kernel(self, **kwargs))
177176
self._kernel_connections[kernel_id] = 0
178177
self.start_watching_activity(kernel_id)
179178
self.log.info("Kernel started: %s" % kernel_id)
@@ -302,12 +301,12 @@ def shutdown_kernel(self, kernel_id, now=False, restart=False):
302301
type=self._kernels[kernel_id].kernel_name
303302
).dec()
304303

305-
return self.super.shutdown_kernel(self, kernel_id, now=now, restart=restart)
304+
return self.pinned_superclass.shutdown_kernel(self, kernel_id, now=now, restart=restart)
306305

307306
async def restart_kernel(self, kernel_id, now=False):
308307
"""Restart a kernel by kernel_id"""
309308
self._check_kernel_id(kernel_id)
310-
await maybe_future(self.super.restart_kernel(self, kernel_id, now=now))
309+
await maybe_future(self.pinned_superclass.restart_kernel(self, kernel_id, now=now))
311310
kernel = self.get_kernel(kernel_id)
312311
# return a Future that will resolve when the kernel has successfully restarted
313312
channel = kernel.connect_shell()
@@ -374,7 +373,7 @@ def kernel_model(self, kernel_id):
374373
def list_kernels(self):
375374
"""Returns a list of kernel_id's of kernels running."""
376375
kernels = []
377-
kernel_ids = self.super.list_kernel_ids(self)
376+
kernel_ids = self.pinned_superclass.list_kernel_ids(self)
378377
for kernel_id in kernel_ids:
379378
model = self.kernel_model(kernel_id)
380379
kernels.append(model)
@@ -485,8 +484,9 @@ def _default_kernel_manager_class(self):
485484
return "jupyter_client.ioloop.AsyncIOLoopKernelManager"
486485

487486
def __init__(self, **kwargs):
488-
self.super = AsyncMultiKernelManager
489-
self.super.__init__(self, **kwargs)
487+
# Pin the superclass to better control the MRO.
488+
self.pinned_superclass = AsyncMultiKernelManager
489+
self.pinned_superclass.__init__(self, **kwargs)
490490
self.last_kernel_activity = utcnow()
491491

492492
async def shutdown_kernel(self, kernel_id, now=False, restart=False):
@@ -505,4 +505,4 @@ async def shutdown_kernel(self, kernel_id, now=False, restart=False):
505505
type=self._kernels[kernel_id].kernel_name
506506
).dec()
507507

508-
return await self.super.shutdown_kernel(self, kernel_id, now=now, restart=restart)
508+
return await self.pinned_superclass.shutdown_kernel(self, kernel_id, now=now, restart=restart)

notebook/utils.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,3 +327,43 @@ def maybe_future(obj):
327327
f.set_result(obj)
328328
return f
329329

330+
331+
def run_sync(maybe_async):
332+
"""If async, runs maybe_async and blocks until it has executed,
333+
possibly creating an event loop.
334+
If not async, just returns maybe_async as it is the result of something
335+
that has already executed.
336+
Parameters
337+
----------
338+
maybe_async : async or non-async object
339+
The object to be executed, if it is async.
340+
Returns
341+
-------
342+
result :
343+
Whatever the async object returns, or the object itself.
344+
"""
345+
if not inspect.isawaitable(maybe_async):
346+
# that was not something async, just return it
347+
return maybe_async
348+
# it is async, we need to run it in an event loop
349+
350+
def wrapped():
351+
create_new_event_loop = False
352+
try:
353+
loop = asyncio.get_event_loop()
354+
except RuntimeError:
355+
create_new_event_loop = True
356+
else:
357+
if loop.is_closed():
358+
create_new_event_loop = True
359+
if create_new_event_loop:
360+
loop = asyncio.new_event_loop()
361+
asyncio.set_event_loop(loop)
362+
try:
363+
result = loop.run_until_complete(maybe_async)
364+
except RuntimeError as e:
365+
if str(e) == 'This event loop is already running':
366+
# just return a Future, hoping that it will be awaited
367+
result = asyncio.ensure_future(maybe_async)
368+
return result
369+
return wrapped()

0 commit comments

Comments
 (0)