Skip to content

Added Logs for get_os_path closes issue #1336

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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