From a984e0771da5db4a14e9ac86a392ad3592b863e5 Mon Sep 17 00:00:00 2001 From: Jayesh Singh Date: Sun, 15 Oct 2023 20:22:07 +0530 Subject: [PATCH] Added Logs for get_os_path closes issue (#1336) Co-authored-by: Steven Silvester --- jupyter_server/services/contents/fileio.py | 10 ++++++---- jupyter_server/services/contents/filemanager.py | 1 - tests/services/contents/test_fileio.py | 4 ++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/jupyter_server/services/contents/fileio.py b/jupyter_server/services/contents/fileio.py index ae9d70fa35..3033ebe3fa 100644 --- a/jupyter_server/services/contents/fileio.py +++ b/jupyter_server/services/contents/fileio.py @@ -15,6 +15,7 @@ from tornado.web import HTTPError from traitlets import Bool from traitlets.config import Configurable +from traitlets.config.configurable import LoggingConfigurable from jupyter_server.utils import ApiPath, to_api_path, to_os_path @@ -165,7 +166,7 @@ def _simple_writing(path, text=True, encoding="utf-8", log=None, **kwargs): fileobj.close() -class FileManagerMixin(Configurable): +class FileManagerMixin(LoggingConfigurable, Configurable): """ Mixin for ContentsAPI classes that interact with the filesystem. @@ -203,7 +204,7 @@ def atomic_writing(self, os_path, *args, **kwargs): Depending on flag 'use_atomic_writing', the wrapper perform an actual atomic writing or simply writes the file (whatever an old exists or not)""" with self.perm_to_403(os_path): - kwargs["log"] = self.log # type:ignore[attr-defined] + kwargs["log"] = self.log if self.use_atomic_writing: with atomic_writing(os_path, *args, **kwargs) as f: yield f @@ -233,7 +234,7 @@ def _copy(self, src, dest): like shutil.copy2, but log errors in copystat """ - copy2_safe(src, dest, log=self.log) # type:ignore[attr-defined] + copy2_safe(src, dest, log=self.log) def _get_os_path(self, path): """Given an API path, return its file system path. @@ -252,6 +253,7 @@ def _get_os_path(self, path): ------ 404: if path is outside root """ + self.log.debug("Reading path from disk: %s", path) root = os.path.abspath(self.root_dir) # type:ignore[attr-defined] # to_os_path is not safe if path starts with a drive, since os.path.join discards first part if os.path.splitdrive(path)[0]: @@ -359,7 +361,7 @@ async def _copy(self, src, dest): like shutil.copy2, but log errors in copystat """ - await async_copy2_safe(src, dest, log=self.log) # type:ignore[attr-defined] + await async_copy2_safe(src, dest, log=self.log) async def _read_notebook(self, os_path, as_version=4, capture_validation_error=None): """Read a notebook from an os path.""" diff --git a/jupyter_server/services/contents/filemanager.py b/jupyter_server/services/contents/filemanager.py index 3558b67402..fe12fb1b7a 100644 --- a/jupyter_server/services/contents/filemanager.py +++ b/jupyter_server/services/contents/filemanager.py @@ -458,7 +458,6 @@ def save(self, model, path=""): raise web.HTTPError(400, "No file type provided") if "content" not in model and model["type"] != "directory": raise web.HTTPError(400, "No file content provided") - os_path = self._get_os_path(path) if not self.allow_hidden and is_hidden(os_path, self.root_dir): diff --git a/tests/services/contents/test_fileio.py b/tests/services/contents/test_fileio.py index 5367bb1a09..0f0cf1bfed 100644 --- a/tests/services/contents/test_fileio.py +++ b/tests/services/contents/test_fileio.py @@ -139,7 +139,7 @@ def test_path_to_invalid(tmpdir): @pytest.mark.skipif(os.name == "nt", reason="test fails on Windows") def test_file_manager_mixin(tmpdir): mixin = FileManagerMixin() - mixin.log = logging.getLogger() # type:ignore[attr-defined] + mixin.log = logging.getLogger() bad_content = tmpdir / "bad_content.ipynb" bad_content.write_text("{}", "utf8") with pytest.raises(HTTPError): @@ -161,7 +161,7 @@ def test_file_manager_mixin(tmpdir): @pytest.mark.skipif(os.name == "nt", reason="test fails on Windows") async def test_async_file_manager_mixin(tmpdir): mixin = AsyncFileManagerMixin() - mixin.log = logging.getLogger() # type:ignore[attr-defined] + mixin.log = logging.getLogger() bad_content = tmpdir / "bad_content.ipynb" bad_content.write_text("{}", "utf8") with pytest.raises(HTTPError):