Skip to content
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

Add support of using custom env variables #1448

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
97747e6
Add support of custom env variables
AnastasiaSliusar Jul 9, 2024
887f522
Fix description of a flag
AnastasiaSliusar Jul 9, 2024
63e634e
fix custom env vars
AnastasiaSliusar Jul 16, 2024
9355a31
Cleaned code
AnastasiaSliusar Jul 31, 2024
2ee8b19
Fix code styling
AnastasiaSliusar Jul 31, 2024
86ddf23
Merge branch 'main' into custom-env-variables
AnastasiaSliusar Jul 31, 2024
ae37186
Fix type
AnastasiaSliusar Aug 5, 2024
93b14be
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 5, 2024
4b04556
Fix types
AnastasiaSliusar Aug 5, 2024
ef07d15
Resolve conflicts
AnastasiaSliusar Aug 5, 2024
084b5ba
Merge branch 'custom-env-variables' of https://github.com/AnastasiaSl…
AnastasiaSliusar Aug 5, 2024
0c49734
Remove custom page config
AnastasiaSliusar Aug 6, 2024
ba231eb
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 6, 2024
56a6b42
Add a test, clean code
AnastasiaSliusar Aug 9, 2024
b0feeba
Merge branch 'custom-env-variables' of https://github.com/AnastasiaSl…
AnastasiaSliusar Aug 9, 2024
6bea16b
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 9, 2024
90418ca
Fix formatting
AnastasiaSliusar Aug 9, 2024
c1c84c8
Merge branch 'custom-env-variables' of https://github.com/AnastasiaSl…
AnastasiaSliusar Aug 9, 2024
9a9958a
Merge remote-tracking branch 'upstream/main' into custom-env-variables
AnastasiaSliusar Aug 9, 2024
f1c3d55
Update a variable name
AnastasiaSliusar Aug 20, 2024
01db90a
Fix lint
AnastasiaSliusar Aug 22, 2024
e51f647
Fix setuping custom env vars
AnastasiaSliusar Aug 28, 2024
4d48ec4
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 28, 2024
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
13 changes: 13 additions & 0 deletions jupyter_server/serverapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,8 @@ def init_settings(
# collapse $HOME to ~
root_dir = "~" + root_dir[len(home) :]

self.allow_custom_env_variables = jupyter_app.allow_custom_env_variables

settings = {
# basics
"log_function": log_request,
Expand Down Expand Up @@ -460,6 +462,7 @@ def init_settings(
"server_root_dir": root_dir,
"jinja2_env": env,
"serverapp": jupyter_app,
"page_config_hook": (self.page_config_hook),
}

# allow custom overrides for the tornado web app.
Expand All @@ -470,6 +473,10 @@ def init_settings(
settings["xsrf_cookie_kwargs"] = {"path": base_url}
return settings

def page_config_hook(self, handler, page_config):
page_config["allow_custom_env_variables"] = self.allow_custom_env_variables
return page_config

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

page_config is a concept specific to JupyterLab. Up until this point, Jupyter Server has been agnostic to it. This hook might need to land in jupyterlab_server instead, which extends jupyter_server and brings the page_config concept to the server.

def init_handlers(self, default_services, settings):
"""Load the (URL pattern, handler) tuples for each component."""
# Order matters. The first handler to match the URL will handle the request.
Expand Down Expand Up @@ -1427,6 +1434,12 @@ def _default_allow_remote(self) -> bool:
""",
)

allow_custom_env_variables = Bool(
False,
config=True,
help="""Allow to use custom env variables""",
)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this trait name and description is too vague.

Since this is specifically about passing custom environment variables to the kernel, I think we need to clarify in this trait and help message that we mean kernel env vars here.

browser = Unicode(
"",
config=True,
Expand Down
1 change: 1 addition & 0 deletions jupyter_server/services/kernels/kernelmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ async def _async_start_kernel( # type:ignore[override]
The name identifying which kernel spec to launch. This is ignored if
an existing kernel is returned, but it may be checked in the future.
"""

if kernel_id is None or kernel_id not in self:
if path is not None:
kwargs["cwd"] = self.cwd_for_path(path, env=kwargs.get("env", {}))
Expand Down
10 changes: 10 additions & 0 deletions jupyter_server/services/sessions/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ async def post(self):
kernel = model.get("kernel", {})
kernel_name = kernel.get("name", None)
kernel_id = kernel.get("id", None)
custom_env_vars = kernel.get("custom_env_vars", {})

if not kernel_id and not kernel_name:
self.log.debug("No kernel specified, using default kernel")
Expand All @@ -93,6 +94,7 @@ async def post(self):
kernel_id=kernel_id,
name=name,
type=mtype,
custom_env_vars=custom_env_vars,
)
except NoSuchKernel:
msg = (
Expand Down Expand Up @@ -152,6 +154,8 @@ async def patch(self, session_id):
changes["name"] = model["name"]
if "type" in model:
changes["type"] = model["type"]
if "custom_env_vars" in model:
changes["custom_env_vars"] = model["custom_env_vars"]
if "kernel" in model:
# Kernel id takes precedence over name.
if model["kernel"].get("id") is not None:
Expand All @@ -160,13 +164,19 @@ async def patch(self, session_id):
raise web.HTTPError(400, "No such kernel: %s" % kernel_id)
changes["kernel_id"] = kernel_id
elif model["kernel"].get("name") is not None:
if "custom_env_vars" in model["kernel"]:
custom_env_vars = model["kernel"]["custom_env_vars"]
else:
custom_env_vars = {}

kernel_name = model["kernel"]["name"]
kernel_id = await sm.start_kernel_for_session(
session_id,
kernel_name=kernel_name,
name=before["name"],
path=before["path"],
type=before["type"],
custom_env_vars=custom_env_vars,
)
changes["kernel_id"] = kernel_id

Expand Down
35 changes: 31 additions & 4 deletions jupyter_server/services/sessions/sessionmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ def __init__(self, *args, **kwargs):
_cursor = None
_connection = None
_columns = {"session_id", "path", "name", "type", "kernel_id"}
_custom_envs = {}

@property
def cursor(self):
Expand Down Expand Up @@ -267,6 +268,7 @@ async def create_session(
type: Optional[str] = None,
kernel_name: Optional[KernelName] = None,
kernel_id: Optional[str] = None,
custom_env_vars: Optional[Dict[str, Any]] = None,
) -> Dict[str, Any]:
"""Creates a session and returns its model

Expand All @@ -283,7 +285,7 @@ async def create_session(
pass
else:
kernel_id = await self.start_kernel_for_session(
session_id, path, name, type, kernel_name
session_id, path, name, type, kernel_name, custom_env_vars
)
record.kernel_id = kernel_id
self._pending_sessions.update(record)
Expand Down Expand Up @@ -319,6 +321,7 @@ async def start_kernel_for_session(
name: Optional[ModelName],
type: Optional[str],
kernel_name: Optional[KernelName],
custom_env_vars: Optional[Dict[str, Any]] = None,
) -> str:
"""Start a new kernel for a given session.

Expand All @@ -335,16 +338,25 @@ async def start_kernel_for_session(
the type of the session
kernel_name : str
the name of the kernel specification to use. The default kernel name will be used if not provided.
custom_env_vars: dict
dictionary of custom env variables
"""
# allow contents manager to specify kernels cwd
kernel_path = await ensure_async(self.contents_manager.get_kernel_path(path=path))

kernel_env = self.get_kernel_env(path, name)

# if we have custom env than we have to add them to available env variables
if custom_env_vars is not None and isinstance(custom_env_vars, dict):
for key, value in custom_env_vars.items():
kernel_env[key] = value

kernel_id = await self.kernel_manager.start_kernel(
path=kernel_path,
kernel_name=kernel_name,
env=kernel_env,
)

self._custom_envs[kernel_id] = custom_env_vars
return cast(str, kernel_id)

async def save_session(self, session_id, path=None, name=None, type=None, kernel_id=None):
Expand Down Expand Up @@ -445,6 +457,7 @@ async def update_session(self, session_id, **kwargs):
and the value replaces the current value in the session
with session_id.
"""

await self.get_session(session_id=session_id)

if not kwargs:
Expand All @@ -464,7 +477,18 @@ async def update_session(self, session_id, **kwargs):
"SELECT path, name, kernel_id FROM session WHERE session_id=?", [session_id]
)
path, name, kernel_id = self.cursor.fetchone()
self.kernel_manager.update_env(kernel_id=kernel_id, env=self.get_kernel_env(path, name))

env = self.get_kernel_env(path, name)

# if we have custom env than we have to add them to available env variables
if self._custom_envs is not None and isinstance(self._custom_envs, dict):
if self._custom_envs[kernel_id] is not None and isinstance(
self._custom_envs[kernel_id], dict
):
for key, value in self._custom_envs[kernel_id].items():
env[key] = value

self.kernel_manager.update_env(kernel_id=kernel_id, env=env)

async def kernel_culled(self, kernel_id: str) -> bool:
"""Checks if the kernel is still considered alive and returns true if its not found."""
Expand Down Expand Up @@ -526,6 +550,9 @@ async def delete_session(self, session_id):
record = KernelSessionRecord(session_id=session_id)
self._pending_sessions.update(record)
session = await self.get_session(session_id=session_id)
await ensure_async(self.kernel_manager.shutdown_kernel(session["kernel"]["id"]))
kernel_id = session["kernel"]["id"]
if kernel_id in self._custom_envs:
del self._custom_envs[kernel_id]
await ensure_async(self.kernel_manager.shutdown_kernel(kernel_id))
self.cursor.execute("DELETE FROM session WHERE session_id=?", (session_id,))
self._pending_sessions.remove(record)
Loading