Skip to content

Commit

Permalink
Added Logs for get_os_path closes issue (#1336)
Browse files Browse the repository at this point in the history
Co-authored-by: Steven Silvester <[email protected]>
  • Loading branch information
jayeshsingh9767 and blink1073 authored Oct 15, 2023
1 parent 89c8de5 commit a984e07
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 7 deletions.
10 changes: 6 additions & 4 deletions jupyter_server/services/contents/fileio.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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]:
Expand Down Expand Up @@ -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."""
Expand Down
1 change: 0 additions & 1 deletion jupyter_server/services/contents/filemanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
4 changes: 2 additions & 2 deletions tests/services/contents/test_fileio.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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):
Expand Down

0 comments on commit a984e07

Please sign in to comment.