Skip to content

Commit

Permalink
fix: on_kernel_start callbacks acculumated after hot reload
Browse files Browse the repository at this point in the history
Introduced in #471
We should remove the on_kernel_start callbacks on a hot reload, but
not remove the ones added by the hot reload itself.
  • Loading branch information
maartenbreddels committed Mar 25, 2024
1 parent 0c33e33 commit 33711d0
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 6 deletions.
32 changes: 32 additions & 0 deletions solara/server/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ def __init__(self, name, default_app_name="Page"):
if reload.reloader.on_change:
raise RuntimeError("Previous reloader still had a on_change attached, no cleanup?")
reload.reloader.on_change = self.on_file_change
# create a snapshot of the current callbacks, so we can remove the ones we added
# so we don't keep adding them after hot reload
self._on_kernel_start_callbacks_before_run = kernel_context._on_kernel_start_callbacks.copy()

self.app_name = default_app_name
if ":" in self.fullname:
Expand Down Expand Up @@ -249,6 +252,35 @@ def reload(self):

solara.lab.toestand.ConnectionStore._type_counter.clear()

# we need to remove callbacks that are added in the app code
# which will be re-executed after the reload and we do not
# want to keep executing the old ones.
keep_on_kernel_start_callbacks = []
for kc in kernel_context._on_kernel_start_callbacks.copy():
callback, path, module = kc
will_reload = False
if module is not None:
module_name = module.__name__
if module_name in reload.reloader.get_reload_module_names():
will_reload = True
elif path is not None:
if str(path.resolve()).startswith(str(self.directory)):
will_reload = True
else:
logger.warning(
"script %s is not in the same directory as the app %s but is using on_kernel_start, "
"this might lead to multiple entries, and might indicate a bug.",
path,
self.directory,
)

if will_reload:
logger.info("reload: Removing on_kernel_start callback: %s (since it will be added when reloaded)", callback)
else:
keep_on_kernel_start_callbacks.append(kc)
kernel_context._on_kernel_start_callbacks.clear()
kernel_context._on_kernel_start_callbacks.extend(keep_on_kernel_start_callbacks)

context_values = list(kernel_context.contexts.values())
# save states into the context so the hot reload will
# keep the same state
Expand Down
39 changes: 35 additions & 4 deletions solara/server/kernel_context.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import asyncio
import dataclasses
import enum
import inspect
import logging
import os
import pickle
import threading
import time
from pathlib import Path
from typing import Any, Callable, Dict, List, Optional, cast
from types import FrameType, ModuleType
from typing import Any, Callable, Dict, List, NamedTuple, Optional, cast

import ipywidgets as widgets
import reacton
Expand Down Expand Up @@ -36,11 +38,40 @@ class PageStatus(enum.Enum):
CLOSED = "closed"


_on_kernel_start_callbacks: List[Callable[[], Optional[Callable[[], None]]]] = []
class _on_kernel_callback_entry(NamedTuple):
f: Callable[[], Optional[Callable[[], None]]]
callpoint: Optional[Path]
module: Optional[ModuleType]


_on_kernel_start_callbacks: List[_on_kernel_callback_entry] = []


def _find_root_module_frame() -> Optional[FrameType]:
# basically the module where the call stack origined from
current_frame = inspect.currentframe()
root_module_frame = None

while current_frame is not None:
if current_frame.f_code.co_name == "<module>":
root_module_frame = current_frame
break
current_frame = current_frame.f_back

return root_module_frame


def on_kernel_start(f: Callable[[], Optional[Callable[[], None]]]):
_on_kernel_start_callbacks.append(f)
root = _find_root_module_frame()
path: Optional[Path] = None
module: Optional[ModuleType] = None
if root is not None:
path_str = inspect.getsourcefile(root)
module = inspect.getmodule(root)
if path_str is not None:
path = Path(path_str)

_on_kernel_start_callbacks.append(_on_kernel_callback_entry(f, path, module))


@dataclasses.dataclass
Expand Down Expand Up @@ -74,7 +105,7 @@ class VirtualKernelContext:

def __post_init__(self):
with self:
for f in _on_kernel_start_callbacks:
for (f, *_) in _on_kernel_start_callbacks:
cleanup = f()
if cleanup:
self.on_close(cleanup)
Expand Down
4 changes: 2 additions & 2 deletions solara/server/reload.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,13 +152,13 @@ def start(self):
self._first = False

def _on_change(self, name):
# used for testing
self.reload_event_next.set()
# flag that we need to reload all modules next time
self.requires_reload = True
# and forward callback
if self.on_change:
self.on_change(name)
# used for testing
self.reload_event_next.set()

def close(self):
self.watcher.close()
Expand Down
33 changes: 33 additions & 0 deletions tests/unit/reload_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import shutil
from pathlib import Path

import solara.server.kernel_context
from solara.server import reload
from solara.server.app import AppScript

HERE = Path(__file__).parent

kernel_start_path = HERE / "solara_test_apps" / "kernel_start.py"


def test_script_reload_component(tmpdir, kernel_context, extra_include_path, no_kernel_context):

target = Path(tmpdir) / "kernel_start.py"
shutil.copy(kernel_start_path, target)
with extra_include_path(str(tmpdir)):
on_kernel_start_callbacks = solara.server.kernel_context._on_kernel_start_callbacks.copy()
app = AppScript(f"{target}")
try:
app.run()
callback = app.routes[0].module.test_callback # type: ignore
assert solara.server.kernel_context._on_kernel_start_callbacks == [*on_kernel_start_callbacks, callback]
prev = solara.server.kernel_context._on_kernel_start_callbacks.copy()
target.touch()
# wait for the event to trigger
reload.reloader.reload_event_next.wait()
app.run()
callback = app.routes[0].module.test_callback # type: ignore
assert solara.server.kernel_context._on_kernel_start_callbacks != prev
assert solara.server.kernel_context._on_kernel_start_callbacks == [*on_kernel_start_callbacks, callback]
finally:
app.close()
14 changes: 14 additions & 0 deletions tests/unit/solara_test_apps/kernel_start.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import solara
import solara.lab


def test_callback():
pass


solara.lab.on_kernel_start(test_callback)


@solara.component
def Page():
solara.Text("Hello, World!")

0 comments on commit 33711d0

Please sign in to comment.