From 6e9256b0641a85baf664e846515085bac2c082a3 Mon Sep 17 00:00:00 2001
From: Kevin Bates <kbates4@gmail.com>
Date: Mon, 25 May 2020 02:48:42 -0700
Subject: [PATCH] Culling: ensure last_activity attr exists before use (#5355)

KernelManager's last_activity attribute is added following the instance's
creation.  Culling (independently) uses this attribute and should ensure
it exists prior to its use, else skip the culling check for that instance.

Fixes #5345
---
 notebook/services/kernels/kernelmanager.py    |  6 +--
 .../kernels/tests/test_kernels_api.py         | 47 +++++++++++++++++++
 2 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/notebook/services/kernels/kernelmanager.py b/notebook/services/kernels/kernelmanager.py
index 83be9b0367..61cbbe58f5 100644
--- a/notebook/services/kernels/kernelmanager.py
+++ b/notebook/services/kernels/kernelmanager.py
@@ -459,9 +459,9 @@ async def cull_kernel_if_idle(self, kernel_id):
         except KeyError:
             return  # KeyErrors are somewhat expected since the kernel can be shutdown as the culling check is made.
 
-        self.log.debug("kernel_id=%s, kernel_name=%s, last_activity=%s",
-                       kernel_id, kernel.kernel_name, kernel.last_activity)
-        if kernel.last_activity is not None:
+        if hasattr(kernel, 'last_activity'):  # last_activity is monkey-patched, so ensure that has occurred
+            self.log.debug("kernel_id=%s, kernel_name=%s, last_activity=%s",
+                           kernel_id, kernel.kernel_name, kernel.last_activity)
             dt_now = utcnow()
             dt_idle = dt_now - kernel.last_activity
             # Compute idle properties
diff --git a/notebook/services/kernels/tests/test_kernels_api.py b/notebook/services/kernels/tests/test_kernels_api.py
index 70ae0e8784..36c441c2c7 100644
--- a/notebook/services/kernels/tests/test_kernels_api.py
+++ b/notebook/services/kernels/tests/test_kernels_api.py
@@ -4,6 +4,7 @@
 import sys
 import time
 
+from requests import HTTPError
 from traitlets.config import Config
 
 from tornado.httpclient import HTTPRequest
@@ -234,3 +235,49 @@ class KernelFilterTest(NotebookTestBase):
     # Sanity check verifying that the configurable was properly set.
     def test_config(self):
         self.assertEqual(self.notebook.kernel_manager.allowed_message_types, ['kernel_info_request'])
+
+
+class KernelCullingTest(NotebookTestBase):
+    """Test kernel culling """
+
+    @classmethod
+    def get_argv(cls):
+        argv = super(KernelCullingTest, cls).get_argv()
+
+        # Enable culling with 2s timeout and 1s intervals
+        argv.extend(['--MappingKernelManager.cull_idle_timeout=2',
+                     '--MappingKernelManager.cull_interval=1',
+                     '--MappingKernelManager.cull_connected=False'])
+        return argv
+
+    def setUp(self):
+        self.kern_api = KernelAPI(self.request,
+                                  base_url=self.base_url(),
+                                  headers=self.auth_headers(),
+                                  )
+
+    def tearDown(self):
+        for k in self.kern_api.list().json():
+            self.kern_api.shutdown(k['id'])
+
+    def test_culling(self):
+        kid = self.kern_api.start().json()['id']
+        ws = self.kern_api.websocket(kid)
+        model = self.kern_api.get(kid).json()
+        self.assertEqual(model['connections'], 1)
+        assert not self.get_cull_status(kid)  # connected, should not be culled
+        ws.close()
+        assert self.get_cull_status(kid)  # not connected, should be culled
+
+    def get_cull_status(self, kid):
+        culled = False
+        for i in range(15):  # Need max of 3s to ensure culling timeout exceeded
+            try:
+                self.kern_api.get(kid)
+            except HTTPError as e:
+                assert e.response.status_code == 404
+                culled = True
+                break
+            else:
+                time.sleep(0.2)
+        return culled