Skip to content

Commit ec700b0

Browse files
committed
Set JPY_SESSION_NAME to full notebook path.
This also add some typing here and there, and extend one of the console warning to log an exception when there is an error. My main concern is that get_kernel_env need to become async. cleanup
1 parent 5965c7f commit ec700b0

File tree

7 files changed

+65
-24
lines changed

7 files changed

+65
-24
lines changed

jupyter_server/base/handlers.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -725,7 +725,7 @@ def write_error(self, status_code, **kwargs):
725725
reply["message"] = "Unhandled error"
726726
reply["reason"] = None
727727
reply["traceback"] = "".join(traceback.format_exception(*exc_info))
728-
self.log.warning("wrote error: %r", reply["message"])
728+
self.log.warning("wrote error: %r", reply["message"], exc_info=True)
729729
self.finish(json.dumps(reply))
730730

731731
def get_login_url(self):

jupyter_server/services/contents/fileio.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
from traitlets import Bool
1717
from traitlets.config import Configurable
1818

19-
from jupyter_server.utils import to_api_path, to_os_path
19+
from jupyter_server.utils import ApiPath, to_api_path, to_os_path
2020

2121

2222
def replace_file(src, dst):
@@ -254,7 +254,7 @@ def _get_os_path(self, path):
254254
404: if path is outside root
255255
"""
256256
root = os.path.abspath(self.root_dir)
257-
os_path = to_os_path(path, root)
257+
os_path = to_os_path(ApiPath(path), root)
258258
if not (os.path.abspath(os_path) + os.path.sep).startswith(root):
259259
raise HTTPError(404, "%s is outside root contents directory" % path)
260260
return os_path

jupyter_server/services/kernels/kernelmanager.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
from collections import defaultdict
1212
from datetime import datetime, timedelta
1313
from functools import partial
14+
from typing import Dict as DictType
15+
from typing import Optional
1416

1517
from jupyter_client.ioloop.manager import AsyncIOLoopKernelManager
1618
from jupyter_client.multikernelmanager import (
@@ -38,7 +40,7 @@
3840

3941
from jupyter_server._tz import isoformat, utcnow
4042
from jupyter_server.prometheus.metrics import KERNEL_CURRENTLY_RUNNING_TOTAL
41-
from jupyter_server.utils import ensure_async, import_item, to_os_path
43+
from jupyter_server.utils import ensure_async, import_item, to_os_path, ApiPath
4244

4345

4446
class MappingKernelManager(MultiKernelManager):
@@ -58,7 +60,7 @@ def _default_kernel_manager_class(self):
5860

5961
_kernel_connections = Dict()
6062

61-
_kernel_ports = Dict()
63+
_kernel_ports: DictType[str, str] = Dict()
6264

6365
_culler_callback = None
6466

@@ -196,12 +198,14 @@ async def _remove_kernel_when_ready(self, kernel_id, kernel_awaitable):
196198
self._kernel_connections.pop(kernel_id, None)
197199
self._kernel_ports.pop(kernel_id, None)
198200

199-
async def _async_start_kernel(self, kernel_id=None, path=None, **kwargs):
201+
async def _async_start_kernel(
202+
self, kernel_id: Optional[str] = None, path: Optional[ApiPath] = None, **kwargs: str
203+
) -> str:
200204
"""Start a kernel for a session and return its kernel_id.
201205
202206
Parameters
203207
----------
204-
kernel_id : uuid
208+
kernel_id : uuid (str)
205209
The uuid to associate the new kernel with. If this
206210
is not None, this kernel will be persistent whenever it is
207211
requested.
@@ -216,6 +220,7 @@ async def _async_start_kernel(self, kernel_id=None, path=None, **kwargs):
216220
if path is not None:
217221
kwargs["cwd"] = self.cwd_for_path(path, env=kwargs.get("env", {}))
218222
if kernel_id is not None:
223+
assert kernel_id is not None
219224
kwargs["kernel_id"] = kernel_id
220225
kernel_id = await self.pinned_superclass._async_start_kernel(self, **kwargs)
221226
self._kernel_connections[kernel_id] = 0
@@ -242,7 +247,7 @@ async def _async_start_kernel(self, kernel_id=None, path=None, **kwargs):
242247
# Initialize culling if not already
243248
if not self._initialized_culler:
244249
self.initialize_culler()
245-
250+
assert kernel_id is not None
246251
return kernel_id
247252

248253
start_kernel = _async_start_kernel

jupyter_server/services/sessions/handlers.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
# Distributed under the terms of the Modified BSD License.
77
import asyncio
88
import json
9+
import os.path
910

1011
try:
1112
from jupyter_client.jsonutil import json_default
@@ -49,10 +50,12 @@ async def post(self):
4950

5051
if "notebook" in model and "path" in model["notebook"]:
5152
self.log.warning("Sessions API changed, see updated swagger docs")
52-
model["path"] = model["notebook"]["path"]
53+
model["path"] = model["notebook"]["name"]
5354
model["type"] = "notebook"
5455

5556
try:
57+
# There is a high chance here that `path` is not a path but
58+
# a unique sesion id
5659
path = model["path"]
5760
except KeyError as e:
5861
raise web.HTTPError(400, "Missing field in JSON data: path") from e

jupyter_server/services/sessions/sessionmanager.py

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import os
55
import pathlib
66
import uuid
7+
from typing import Dict
78

89
try:
910
import sqlite3
@@ -12,7 +13,7 @@
1213
from pysqlite2 import dbapi2 as sqlite3 # type:ignore[no-redef]
1314

1415
from dataclasses import dataclass, fields
15-
from typing import Union
16+
from typing import Optional, Union
1617

1718
from tornado import web
1819
from traitlets import Instance, TraitError, Unicode, validate
@@ -39,8 +40,8 @@ class KernelSessionRecord:
3940
associated with them.
4041
"""
4142

42-
session_id: Union[None, str] = None
43-
kernel_id: Union[None, str] = None
43+
session_id: Optional[str] = None
44+
kernel_id: Optional[str] = None
4445

4546
def __eq__(self, other: object) -> bool:
4647
if isinstance(other, KernelSessionRecord):
@@ -243,14 +244,21 @@ async def session_exists(self, path):
243244
exists = True
244245
return exists
245246

246-
def new_session_id(self):
247+
def new_session_id(self) -> str:
247248
"Create a uuid for a new session"
248249
return str(uuid.uuid4())
249250

250251
async def create_session(
251252
self, path=None, name=None, type=None, kernel_name=None, kernel_id=None
252253
):
253-
"""Creates a session and returns its model"""
254+
"""Creates a session and returns its model
255+
256+
name: str
257+
Usually the model name, like the filename associated with current
258+
kernel.
259+
260+
261+
"""
254262
session_id = self.new_session_id()
255263
record = KernelSessionRecord(session_id=session_id)
256264
self._pending_sessions.update(record)
@@ -268,15 +276,34 @@ async def create_session(
268276
self._pending_sessions.remove(record)
269277
return result
270278

271-
def get_kernel_env(self, path):
272-
"""Return the environment variables that need to be set in the kernel"""
279+
async def get_kernel_env(self, path: str, name: Optional[str] = None) -> Dict[str, str]:
280+
"""Return the environment variables that need to be set in the kernel
281+
282+
name: str
283+
Here the name is likely to be the name of the associated file
284+
with the current kernel at startup time.
285+
286+
287+
"""
288+
if name is not None:
289+
session_dir = await self.contents_manager.get_kernel_path(path=path)
290+
cwd = self.kernel_manager.cwd_for_path(path)
291+
path = os.path.join(cwd, session_dir, name)
292+
assert isinstance(path, str)
273293
return {**os.environ, "JPY_SESSION_NAME": path}
274294

275295
async def start_kernel_for_session(self, session_id, path, name, type, kernel_name):
276-
"""Start a new kernel for a given session."""
296+
"""Start a new kernel for a given session.
297+
298+
name: str
299+
Usually the model name, like the filename associated with current
300+
kernel.
301+
302+
"""
277303
# allow contents manager to specify kernels cwd
278304
kernel_path = await ensure_async(self.contents_manager.get_kernel_path(path=path))
279-
kernel_env = self.get_kernel_env(path)
305+
306+
kernel_env = await self.get_kernel_env(path, name)
280307
kernel_id = await self.kernel_manager.start_kernel(
281308
path=kernel_path,
282309
kernel_name=kernel_name,

jupyter_server/utils.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import sys
99
import warnings
1010
from contextlib import contextmanager
11+
from typing import NewType
1112
from urllib.parse import urljoin # noqa: F401
1213
from urllib.parse import SplitResult, quote, unquote, urlparse, urlsplit, urlunsplit
1314
from urllib.request import pathname2url # noqa: F401
@@ -18,6 +19,8 @@
1819
from tornado.httpclient import AsyncHTTPClient, HTTPClient, HTTPRequest
1920
from tornado.netutil import Resolver
2021

22+
ApiPath = NewType("ApiPath", str)
23+
2124

2225
def url_path_join(*pieces):
2326
"""Join components of url into a relative url
@@ -102,19 +105,19 @@ def samefile_simple(path, other_path):
102105
return path.lower() == other_path.lower() and path_stat == other_path_stat
103106

104107

105-
def to_os_path(path, root=""):
108+
def to_os_path(path: ApiPath, root: str = "") -> str:
106109
"""Convert an API path to a filesystem path
107110
108111
If given, root will be prepended to the path.
109112
root must be a filesystem path already.
110113
"""
111-
parts = path.strip("/").split("/")
114+
parts = str(path).strip("/").split("/")
112115
parts = [p for p in parts if p != ""] # remove duplicate splits
113-
path = os.path.join(root, *parts)
114-
return os.path.normpath(path)
116+
path_ = os.path.join(root, *parts)
117+
return os.path.normpath(path_)
115118

116119

117-
def to_api_path(os_path, root=""):
120+
def to_api_path(os_path: str, root: str = "") -> ApiPath:
118121
"""Convert a filesystem path to an API path
119122
120123
If given, root will be removed from the path.
@@ -125,7 +128,7 @@ def to_api_path(os_path, root=""):
125128
parts = os_path.strip(os.path.sep).split(os.path.sep)
126129
parts = [p for p in parts if p != ""] # remove duplicate splits
127130
path = "/".join(parts)
128-
return path
131+
return ApiPath(path)
129132

130133

131134
def check_version(v, check):

tests/utils.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import json
2+
from typing import NewType
23

34
from tornado.httpclient import HTTPClientError
45
from tornado.web import HTTPError
@@ -10,6 +11,8 @@
1011
"display_name": "Test kernel",
1112
}
1213

14+
ApiPath = NewType("ApiPath", str)
15+
1316

1417
def mkdir(tmp_path, *parts):
1518
path = tmp_path.joinpath(*parts)

0 commit comments

Comments
 (0)