Merge pull request #41 from Anaconda-Platform/conda-caching

More robust handling of conda info, bad conda

nb_conda_kernels/manager.py

@@ -2,41 +2,70 @@
 import json
 import subprocess
 import sys
+import time
 
 from os.path import exists, join, split, dirname, abspath
 
-from jupyter_client.kernelspec import KernelSpecManager, KernelSpec, NATIVE_KERNEL_NAME
+from jupyter_client.kernelspec import (
+    KernelSpecManager,
+    KernelSpec,
+    NATIVE_KERNEL_NAME,
+)
+
+CACHE_TIMEOUT = 60
 
 
 class CondaKernelSpecManager(KernelSpecManager):
-    """A custom KernelSpecManager able to search for conda environments and
-    create kernelspecs for them.
+    """ A custom KernelSpecManager able to search for conda environments and
+        create kernelspecs for them.
     """
     def __init__(self, **kwargs):
         super(CondaKernelSpecManager, self).__init__(**kwargs)
-        self.conda_info = None
-        specs = self.find_kernel_specs() or {}
-        self.log.info("[nb_conda_kernels] enabled, {} kernels found".format(
-            len(specs)
-        ))
-        self.log.debug("\n{}".format(
-            "\n".join(["- {}: {}".format(*spec) for spec in specs.items()])
-        ))
 
+        self._conda_info_cache = None
+        self._conda_info_cache_expiry = None
+
+        self._conda_kernels_cache = None
+        self._conda_kernels_cache_expiry = None
+
+        self.log.info("[nb_conda_kernels] enabled, %s kernels found",
+                      len(self._conda_kspecs))
+
+    @property
     def _conda_info(self):
-        "Get and parse the whole conda information"
-        p = subprocess.check_output(["conda", "info", "--json"]
-                                    ).decode("utf-8")
-        conda_info = json.loads(p)
+        """ Get and parse the whole conda information output
 
-        return conda_info
+            Caches the information for CACHE_TIMEOUT seconds, as this is
+            relatively expensive.
+        """
+
+        expiry = self._conda_info_cache_expiry
+
+        if expiry is None or expiry < time.time():
+            self.log.debug("[nb_conda_kernels] refreshing conda info")
+            try:
+                p = subprocess.check_output(["conda", "info", "--json"]
+                                            ).decode("utf-8")
+                conda_info = json.loads(p)
+            except Exception as err:
+                conda_info = None
+                self.log.error("[nb_conda_kernels] couldn't call conda:\n%s",
+                               err)
+            self._conda_info_cache = conda_info
+            self._conda_info_cache_expiry = time.time() + CACHE_TIMEOUT
+
+        return self._conda_info_cache
 
     def _all_envs(self):
-        """Find the all the executables for each env where jupyter is installed.
+        """ Find the all the executables for each env where jupyter is
+            installed.
+
+            Returns a dict with the env names as keys and info about the kernel
+            specs, including the paths to the lang executable in each env as
+            value if jupyter is installed in that env.
 
-        Returns a dict with the env names as keys and info about the kernel specs,
-        including the paths to the lang executable in each env as value if jupyter
-        is installed in that env.
+            Caches the information for CACHE_TIMEOUT seconds, as this is
+            relatively expensive.
         """
         # play safe with windows
         if sys.platform.startswith('win'):
@@ -49,7 +78,8 @@ def _all_envs(self):
             jupyter = join("bin", "jupyter")
 
         def get_paths_by_env(display_prefix, language_key, language_exe, envs):
-            "Get a dict with name_env:info for kernel executables"
+            """ Get a dict with name_env:info for kernel executables
+            """
             language_envs = {}
             for base in envs:
                 exe_path = join(base, language_exe)
@@ -69,25 +99,28 @@ def get_paths_by_env(display_prefix, language_key, language_exe, envs):
 
         # Get the python envs
         python_envs = get_paths_by_env("Python", "py", python,
-                                       self.conda_info["envs"])
+                                       self._conda_info["envs"])
         all_envs.update(python_envs)
 
         # Get the R envs
-        r_envs = get_paths_by_env("R", "r", r, self.conda_info["envs"])
+        r_envs = get_paths_by_env("R", "r", r, self._conda_info["envs"])
         all_envs.update(r_envs)
 
         # We also add the root prefix into the soup
-        root_prefix = join(self.conda_info["root_prefix"], jupyter)
+        root_prefix = join(self._conda_info["root_prefix"], jupyter)
         if exists(root_prefix):
             all_envs.update({
                 'conda-root-py': {
                     'display_name': 'Python [conda root]',
-                    'executable': join(self.conda_info["root_prefix"], python),
+                    'executable': join(self._conda_info["root_prefix"],
+                                       python),
                     'language_key': 'py',
                 }
             })
-        # Use Jupyter's default kernel name ('python2' or 'python3') for current env
-        if exists(join(sys.prefix, jupyter)) and exists(join(sys.prefix, python)):
+        # Use Jupyter's default kernel name ('python2' or 'python3') for
+        # current env
+        if exists(join(sys.prefix, jupyter)) and exists(join(sys.prefix,
+                                                             python)):
             all_envs.update({
                 NATIVE_KERNEL_NAME: {
                     'display_name': 'Python [default]',
@@ -98,8 +131,26 @@ def get_paths_by_env(display_prefix, language_key, language_exe, envs):
 
         return all_envs
 
+    @property
     def _conda_kspecs(self):
-        "Create a kernelspec for each of the envs where jupyter is installed"
+        """ Get (or refresh) the cache of conda kernels
+        """
+        if self._conda_info is None:
+            return {}
+
+        if (
+            self._conda_kernels_cache_expiry is None or
+            self._conda_kernels_cache_expiry < time.time()
+           ):
+            self.log.debug("[nb_conda_kernels] refreshing conda kernelspecs")
+            self._conda_kernels_cache = self._load_conda_kspecs()
+            self._conda_kernels_cache_expiry = time.time() + CACHE_TIMEOUT
+
+        return self._conda_kernels_cache
+
+    def _load_conda_kspecs(self):
+        """ Create a kernelspec for each of the envs where jupyter is installed
+        """
         kspecs = {}
         for name, info in self._all_envs().items():
             executable = info['executable']
@@ -117,7 +168,7 @@ def _conda_kspecs(self):
                  }
             elif info['language_key'] == 'r':
                 kspec = {
-                    "argv": [executable, "--quiet", "-e", "IRkernel::main()",
+                    "argv": [executable, "--slave", "-e", "IRkernel::main()",
                              "--args", "{connection_file}"],
                     "display_name": display_name,
                     "language": "R",
@@ -133,30 +184,27 @@ def _conda_kspecs(self):
         return kspecs
 
     def find_kernel_specs(self):
-        """Returns a dict mapping kernel names to resource directories.
+        """ Returns a dict mapping kernel names to resource directories.
 
-        The update process also add the resource dir for the conda
-        environments.
+            The update process also adds the resource dir for the conda
+            environments.
         """
         kspecs = super(CondaKernelSpecManager, self).find_kernel_specs()
 
-        # update conda info
-        self.conda_info = self._conda_info()
-
         # add conda envs kernelspecs
-        kspecs.update(self._conda_kspecs())
-
+        kspecs.update({name: spec.resource_dir
+                       for name, spec
+                       in self._conda_kspecs.items()})
         return kspecs
 
     def get_kernel_spec(self, kernel_name):
-        """Returns a :class:`KernelSpec` instance for the given kernel_name.
+        """ Returns a :class:`KernelSpec` instance for the given kernel_name.
 
-        Additionally, conda kernelspecs are generated on the fly accordingly
-        with the detected envitonments.
+            Additionally, conda kernelspecs are generated on the fly
+            accordingly with the detected envitonments.
         """
-        conda_kspecs = self._conda_kspecs()
-        if kernel_name in conda_kspecs:
-            return conda_kspecs[kernel_name]
-        else:
-            return super(CondaKernelSpecManager, self).get_kernel_spec(
-                kernel_name)
+
+        return (
+            self._conda_kspecs.get(kernel_name) or
+            super(CondaKernelSpecManager, self).get_kernel_spec(kernel_name)
+        )

nb_conda_kernels/tests/test_api.py

@@ -0,0 +1,54 @@
+from subprocess import check_output, CalledProcessError
+
+from notebook.services.kernelspecs.tests import test_kernelspecs_api
+
+try:
+    from unittest.mock import patch
+except ImportError:
+    from mock import patch  # py2
+
+
+CONDA_INFO_ARGS = ["conda", "info", "--json"]
+
+
+class APITest(test_kernelspecs_api.APITest):
+    """ Run all the upstream tests. Assumes:
+        - ipykernel is installed in the root
+        - r is installed in the environment under test
+    """
+
+    def test_has_root_py(self):
+        model = self.ks_api.list().json()
+        self.assertIn("conda-root-py", model["kernelspecs"].keys())
+
+    def test_has_r(self):
+        model = self.ks_api.list().json()
+        self.assertIn("ir", model["kernelspecs"].keys())
+
+
+class BadCondaAPITest(test_kernelspecs_api.APITest):
+    @classmethod
+    def setup_class(cls):
+        def _mock_check_output(cmd, *args, **kwargs):
+            if cmd == CONDA_INFO_ARGS:
+                raise CalledProcessError("bad conda")
+
+            return check_output(cmd, *args, **kwargs)
+
+        cls.cond_info_patch = patch("subprocess.check_output",
+                                    _mock_check_output)
+        cls.cond_info_patch.start()
+        super(BadCondaAPITest, cls).setup_class()
+
+    @classmethod
+    def teardown_class(cls):
+        super(BadCondaAPITest, cls).teardown_class()
+        cls.cond_info_patch.stop()
+
+    def test_no_conda_kernels(self):
+        model = self.ks_api.list().json()
+        self.assertEquals(
+            [],
+            [name for name in model["kernelspecs"].keys()
+             if name.startswith("conda-")]
+        )

nb_conda_kernels/tests/test_notebook.py

@@ -31,7 +31,8 @@ class NBCondaKernelsTestController(jstest.JSController):
     """
     def __init__(self, section, *args, **kwargs):
         extra_args = kwargs.pop('extra_args', None)
-        super(NBCondaKernelsTestController, self).__init__(section, *args, **kwargs)
+        super(NBCondaKernelsTestController, self).__init__(section, *args,
+                                                           **kwargs)
         self.xunit = True
 
         test_cases = glob.glob(os.path.join(here, 'js', 'test_notebook_*.js'))

package.json

@@ -1,11 +1,11 @@
 {
   "name": "nb_conda_kernels",
-  "version": "1.0.3",
+  "version": "2.0.0",
   "description": "Launch Jupyter kernels for any installed conda environment",
   "main": "index.js",
   "scripts": {
     "test": "python -m nose nb_conda_kernels.tests",
-    "lint": "flake8 setup.py nb_conda_kernels"
+    "lint": "npm run lint && flake8 setup.py nb_conda_kernels"
   },
   "repository": {
     "type": "git",
@@ -20,13 +20,5 @@
   "devDependencies": {
     "casperjs": "^1.1.1",
     "phantomjs-prebuilt": "^2.1.7"
-  },
-  "eslintConfig": {
-    "strict": true,
-    "env": {
-      "browser": true,
-      "node": true,
-      "phantomjs": true
-    }
   }
 }