Merge branch 'main' into authorization

.github/workflows/python-linux.yml

@@ -72,7 +72,7 @@ jobs:
           ./test_install/bin/python -m pip install -U pip
           ./test_install/bin/python -m pip install ".[test]"
           pushd test_install
-          ./bin/pytest --pyargs jupyter_server
+          ./bin/pytest --pyargs jupyter_server --capture=no
           popd
       - name: Test the docs
         run: |

CHANGELOG.md

@@ -4,6 +4,22 @@ All notable changes to this project will be documented in this file.
 
 <!-- <START NEW CHANGELOG ENTRY> -->
 
+## 1.13.4
+
+([Full Changelog](https://github.com/jupyter-server/jupyter_server/compare/v1.13.3...d2015290b80bfdaa6ebb990cdccc0155921696f5))
+
+### Bugs fixed
+
+- Fix nbconvert handler run_sync() [#667](https://github.com/jupyter-server/jupyter_server/pull/667) ([@davidbrochart](https://github.com/davidbrochart))
+
+### Contributors to this release
+
+([GitHub contributors page for this release](https://github.com/jupyter-server/jupyter_server/graphs/contributors?from=2022-01-14&to=2022-01-21&type=c))
+
+[@codecov-commenter](https://github.com/search?q=repo%3Ajupyter-server%2Fjupyter_server+involves%3Acodecov-commenter+updated%3A2022-01-14..2022-01-21&type=Issues) | [@davidbrochart](https://github.com/search?q=repo%3Ajupyter-server%2Fjupyter_server+involves%3Adavidbrochart+updated%3A2022-01-14..2022-01-21&type=Issues) | [@Zsailer](https://github.com/search?q=repo%3Ajupyter-server%2Fjupyter_server+involves%3AZsailer+updated%3A2022-01-14..2022-01-21&type=Issues)
+
+<!-- <END NEW CHANGELOG ENTRY> -->
+
 ## 1.13.3
 
 ([Full Changelog](https://github.com/jupyter-server/jupyter_server/compare/v1.13.2...ee01e1955c8881b46075c78f1fbc932fa234bc72))
@@ -22,8 +38,6 @@ All notable changes to this project will be documented in this file.
 
 [@Zsailer](https://github.com/search?q=repo%3Ajupyter-server%2Fjupyter_server+involves%3AZsailer+updated%3A2022-01-12..2022-01-14&type=Issues)
 
-<!-- <END NEW CHANGELOG ENTRY> -->
-
 ## 1.13.2
 
 ([Full Changelog](https://github.com/jupyter-server/jupyter_server/compare/v1.13.1...362d100ff24c1da7ef4cbd171c213e9570e8c289))

jupyter_server/pytest_plugin.py

@@ -476,6 +476,20 @@ def jp_cleanup_subprocesses(jp_serverapp):
     async def _():
         terminal_cleanup = jp_serverapp.web_app.settings["terminal_manager"].terminate_all
         kernel_cleanup = jp_serverapp.kernel_manager.shutdown_all
+
+        async def kernel_cleanup_steps():
+            # Try a graceful shutdown with a timeout
+            try:
+                await asyncio.wait_for(kernel_cleanup(), timeout=15.0)
+            except asyncio.TimeoutError:
+                # Now force a shutdown
+                try:
+                    await asyncio.wait_for(kernel_cleanup(now=True), timeout=15.0)
+                except asyncio.TimeoutError:
+                    print(Exception("Kernel never shutdown!"))
+            except Exception as e:
+                print(e)
+
         if asyncio.iscoroutinefunction(terminal_cleanup):
             try:
                 await terminal_cleanup()
@@ -487,10 +501,7 @@ async def _():
             except Exception as e:
                 print(e)
         if asyncio.iscoroutinefunction(kernel_cleanup):
-            try:
-                await kernel_cleanup()
-            except Exception as e:
-                print(e)
+            await kernel_cleanup_steps()
         else:
             try:
                 kernel_cleanup()

jupyter_server/services/contents/filemanager.py

@@ -155,6 +155,28 @@ def is_hidden(self, path):
         os_path = self._get_os_path(path=path)
         return is_hidden(os_path, self.root_dir)
 
+    def is_writable(self, path):
+        """Does the API style path correspond to a writable directory or file?
+
+        Parameters
+        ----------
+        path : string
+            The path to check. This is an API path (`/` separated,
+            relative to root_dir).
+
+        Returns
+        -------
+        hidden : bool
+            Whether the path exists and is writable.
+        """
+        path = path.strip("/")
+        os_path = self._get_os_path(path=path)
+        try:
+            return os.access(os_path, os.W_OK)
+        except OSError:
+            self.log.error("Failed to check write permissions on %s", os_path)
+            return False
+
     def file_exists(self, path):
         """Returns True if the file exists, else returns False.
 
@@ -251,12 +273,8 @@ def _base_model(self, path):
         model["format"] = None
         model["mimetype"] = None
         model["size"] = size
+        model["writable"] = self.is_writable(path)
 
-        try:
-            model["writable"] = os.access(os_path, os.W_OK)
-        except OSError:
-            self.log.error("Failed to check write permissions on %s", os_path)
-            model["writable"] = False
         return model
 
     def _dir_model(self, path, content=True):
@@ -514,10 +532,12 @@ def is_non_empty_dir(os_path):
                 # deleting non-empty files. See Github issue 3631.
                 raise web.HTTPError(400, u"Directory %s not empty" % os_path)
             if _check_trash(os_path):
-                self.log.debug("Sending %s to trash", os_path)
                 # Looking at the code in send2trash, I don't think the errors it
                 # raises let us distinguish permission errors from other errors in
-                # code. So for now, just let them all get logged as server errors.
+                # code. So for now, the "look before you leap" approach is used.
+                if not self.is_writable(path):
+                    raise web.HTTPError(403, u"Permission denied: %s" % path)
+                self.log.debug("Sending %s to trash", os_path)
                 send2trash(os_path)
                 return
             else:
@@ -842,10 +862,12 @@ async def is_non_empty_dir(os_path):
                 # deleting non-empty files. See Github issue 3631.
                 raise web.HTTPError(400, u"Directory %s not empty" % os_path)
             if await _check_trash(os_path):
-                self.log.debug("Sending %s to trash", os_path)
                 # Looking at the code in send2trash, I don't think the errors it
                 # raises let us distinguish permission errors from other errors in
-                # code. So for now, just let them all get logged as server errors.
+                # code. So for now, the "look before you leap" approach is used.
+                if not self.is_writable(path):
+                    raise web.HTTPError(403, u"Permission denied: %s" % path)
+                self.log.debug("Sending %s to trash", os_path)
                 send2trash(os_path)
                 return
             else:

jupyter_server/tests/services/kernels/test_api.py

@@ -1,18 +1,18 @@
 import json
+import os
 import time
 
+import jupyter_client
 import pytest
 import tornado
 from jupyter_client.kernelspec import NATIVE_KERNEL_NAME
 from tornado.httpclient import HTTPClientError
 
 from ...utils import expected_http_error
-from jupyter_server.services.kernels.kernelmanager import AsyncMappingKernelManager
 from jupyter_server.utils import url_path_join
 
 
-class DummyMappingKernelManager(AsyncMappingKernelManager):
-    """A no-op subclass to use in a fixture"""
+TEST_TIMEOUT = 20
 
 
 @pytest.fixture
@@ -27,22 +27,37 @@ async def _(kernel_id):
     return _
 
 
-@pytest.fixture(
-    params=["MappingKernelManager", "AsyncMappingKernelManager", "DummyMappingKernelManager"]
-)
-def jp_argv(request):
-    if request.param == "DummyMappingKernelManager":
-        extra = []
-        if hasattr(AsyncMappingKernelManager, "use_pending_kernels"):
-            extra = ["--AsyncMappingKernelManager.use_pending_kernels=True"]
-        return [
-            "--ServerApp.kernel_manager_class=jupyter_server.tests.services.kernels.test_api."
-            + request.param
-        ] + extra
-    return [
-        "--ServerApp.kernel_manager_class=jupyter_server.services.kernels.kernelmanager."
-        + request.param
-    ]
+configs = [
+    {
+        "ServerApp": {
+            "kernel_manager_class": "jupyter_server.services.kernels.kernelmanager.MappingKernelManager"
+        }
+    },
+    {
+        "ServerApp": {
+            "kernel_manager_class": "jupyter_server.services.kernels.kernelmanager.AsyncMappingKernelManager"
+        }
+    },
+]
+
+
+# Pending kernels was released in Jupyter Client 7.1
+# It is currently broken on Windows (Jan 2022). When fixed, we can remove the Windows check.
+# See https://github.com/jupyter-server/jupyter_server/issues/672
+if os.name != "nt" and jupyter_client._version.version_info >= (7, 1):
+    # Add a pending kernels condition
+    c = {
+        "ServerApp": {
+            "kernel_manager_class": "jupyter_server.services.kernels.kernelmanager.AsyncMappingKernelManager"
+        },
+        "AsyncMappingKernelManager": {"use_pending_kernels": True},
+    }
+    configs.append(c)
+
+
+@pytest.fixture(params=configs)
+def jp_server_config(request):
+    return request.param
 
 
 async def test_no_kernels(jp_fetch):
@@ -51,6 +66,7 @@ async def test_no_kernels(jp_fetch):
     assert kernels == []
 
 
+@pytest.mark.timeout(TEST_TIMEOUT)
 async def test_default_kernels(jp_fetch, jp_base_url, jp_cleanup_subprocesses):
     r = await jp_fetch("api", "kernels", method="POST", allow_nonstandard_methods=True)
     kernel = json.loads(r.body.decode())
@@ -66,6 +82,7 @@ async def test_default_kernels(jp_fetch, jp_base_url, jp_cleanup_subprocesses):
     await jp_cleanup_subprocesses()
 
 
+@pytest.mark.timeout(TEST_TIMEOUT)
 async def test_main_kernel_handler(
     jp_fetch, jp_base_url, jp_cleanup_subprocesses, jp_serverapp, pending_kernel_is_ready
 ):
@@ -144,6 +161,7 @@ async def test_main_kernel_handler(
     await jp_cleanup_subprocesses()
 
 
+@pytest.mark.timeout(TEST_TIMEOUT)
 async def test_kernel_handler(jp_fetch, jp_cleanup_subprocesses, pending_kernel_is_ready):
     # Create a kernel
     r = await jp_fetch(
@@ -191,6 +209,7 @@ async def test_kernel_handler(jp_fetch, jp_cleanup_subprocesses, pending_kernel_
     await jp_cleanup_subprocesses()
 
 
+@pytest.mark.timeout(TEST_TIMEOUT)
 async def test_kernel_handler_startup_error(
     jp_fetch, jp_cleanup_subprocesses, jp_serverapp, jp_kernelspecs
 ):
@@ -202,6 +221,7 @@ async def test_kernel_handler_startup_error(
         await jp_fetch("api", "kernels", method="POST", body=json.dumps({"name": "bad"}))
 
 
+@pytest.mark.timeout(TEST_TIMEOUT)
 async def test_kernel_handler_startup_error_pending(
     jp_fetch, jp_ws_fetch, jp_cleanup_subprocesses, jp_serverapp, jp_kernelspecs
 ):
@@ -217,6 +237,7 @@ async def test_kernel_handler_startup_error_pending(
         await jp_ws_fetch("api", "kernels", kid, "channels")
 
 
+@pytest.mark.timeout(TEST_TIMEOUT)
 async def test_connection(
     jp_fetch, jp_ws_fetch, jp_http_port, jp_auth_header, jp_cleanup_subprocesses
 ):

jupyter_server/tests/services/kernels/test_cull.py

@@ -1,39 +1,49 @@
 import asyncio
 import json
+import os
 import platform
 
+import jupyter_client
 import pytest
 from tornado.httpclient import HTTPClientError
 from traitlets.config import Config
 
 
-@pytest.fixture(params=["MappingKernelManager", "AsyncMappingKernelManager"])
-def jp_argv(request):
-    return [
-        "--ServerApp.kernel_manager_class=jupyter_server.services.kernels.kernelmanager."
-        + request.param
-    ]
-
-
 CULL_TIMEOUT = 30 if platform.python_implementation() == "PyPy" else 5
 CULL_INTERVAL = 1
 
 
-@pytest.fixture
-def jp_server_config():
-    return Config(
-        {
-            "ServerApp": {
-                "MappingKernelManager": {
-                    "cull_idle_timeout": CULL_TIMEOUT,
-                    "cull_interval": CULL_INTERVAL,
-                    "cull_connected": False,
+@pytest.mark.parametrize(
+    "jp_server_config",
+    [
+        # Test the synchronous case
+        Config(
+            {
+                "ServerApp": {
+                    "kernel_manager_class": "jupyter_server.services.kernels.kernelmanager.MappingKernelManager",
+                    "MappingKernelManager": {
+                        "cull_idle_timeout": CULL_TIMEOUT,
+                        "cull_interval": CULL_INTERVAL,
+                        "cull_connected": False,
+                    },
                 }
             }
-        }
-    )
-
-
+        ),
+        # Test the async case
+        Config(
+            {
+                "ServerApp": {
+                    "kernel_manager_class": "jupyter_server.services.kernels.kernelmanager.AsyncMappingKernelManager",
+                    "AsyncMappingKernelManager": {
+                        "cull_idle_timeout": CULL_TIMEOUT,
+                        "cull_interval": CULL_INTERVAL,
+                        "cull_connected": False,
+                    },
+                }
+            }
+        ),
+    ],
+)
 async def test_cull_idle(jp_fetch, jp_ws_fetch, jp_cleanup_subprocesses):
     r = await jp_fetch("api", "kernels", method="POST", allow_nonstandard_methods=True)
     kernel = json.loads(r.body.decode())
@@ -53,14 +63,36 @@ async def test_cull_idle(jp_fetch, jp_ws_fetch, jp_cleanup_subprocesses):
     await jp_cleanup_subprocesses()
 
 
+# Pending kernels was released in Jupyter Client 7.1
+# It is currently broken on Windows (Jan 2022). When fixed, we can remove the Windows check.
+# See https://github.com/jupyter-server/jupyter_server/issues/672
+@pytest.mark.skipif(
+    os.name == "nt" or jupyter_client._version.version_info < (7, 1),
+    reason="Pending kernels require jupyter_client >= 7.1 on non-Windows",
+)
+@pytest.mark.parametrize(
+    "jp_server_config",
+    [
+        Config(
+            {
+                "ServerApp": {
+                    "kernel_manager_class": "jupyter_server.services.kernels.kernelmanager.AsyncMappingKernelManager",
+                    "AsyncMappingKernelManager": {
+                        "cull_idle_timeout": CULL_TIMEOUT,
+                        "cull_interval": CULL_INTERVAL,
+                        "cull_connected": False,
+                        "default_kernel_name": "bad",
+                        "use_pending_kernels": True,
+                    },
+                }
+            }
+        )
+    ],
+)
+@pytest.mark.timeout(30)
 async def test_cull_dead(
     jp_fetch, jp_ws_fetch, jp_serverapp, jp_cleanup_subprocesses, jp_kernelspecs
 ):
-    if not hasattr(jp_serverapp.kernel_manager, "use_pending_kernels"):
-        return
-
-    jp_serverapp.kernel_manager.use_pending_kernels = True
-    jp_serverapp.kernel_manager.default_kernel_name = "bad"
     r = await jp_fetch("api", "kernels", method="POST", allow_nonstandard_methods=True)
     kernel = json.loads(r.body.decode())
     kid = kernel["id"]