Skip to content

Commit

Permalink
fix: hot reload was broken
Browse files Browse the repository at this point in the history
We now also pickle the state before and after the reload
  • Loading branch information
maartenbreddels committed Jan 20, 2023
1 parent 4395220 commit 478d19c
Show file tree
Hide file tree
Showing 6 changed files with 174 additions and 76 deletions.
62 changes: 32 additions & 30 deletions solara/server/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
state_directory = Path(".") / "states"
state_directory.mkdir(exist_ok=True)

reload.reloader.start()


class AppType(str, Enum):
SCRIPT = "script"
Expand Down Expand Up @@ -91,6 +93,7 @@ def __init__(self, name, default_app_name="Page"):
warn_is_widget()

def _execute(self):
logger.info("Executing %s", self.name)
app = None
local_scope = {
"display": display,
Expand Down Expand Up @@ -270,7 +273,13 @@ def reload(self):
with context:
# we save the state for when the app reruns, so we stay in the same state.
# (e.g. button clicks, chosen options etc)
context.state = render_context.state_get()
# for instance a dataframe, needs to be pickled, because after the pandas
# module is reloaded, it's a different class type
logger.info("pickling state: %s", render_context.state_get())
try:
context.state = pickle.dumps(render_context.state_get())
except Exception as e:
logger.warning("Could not pickle state, next render the state will be lost: %s", e)
# clear/cleanup the render_context, so during reload we start
# from scratch
context.app_object = None
Expand Down Expand Up @@ -417,6 +426,9 @@ def _run_app(

# app.signal_hook_install()
main_object = app_script.run()
app_state = pickle.loads(app_state) if app_state is not None else None
if app_state:
logger.info("Restoring state: %r", app_state)

context = get_current_context()
container = context.container
Expand Down Expand Up @@ -464,35 +476,25 @@ def load_app_widget(app_state, app_script: AppScript, pathname: str):
try:
render_context = context.app_object
with context:
with reload.reloader.watch():
while True:
# reloading might take in extra dependencies, so the reload happens first
if reload.reloader.requires_reload:
reload.reloader.reload()
# reload before starting app, because we may load state using pickle
# if we do that before reloading, the classes are not compatible:
app_state = app_state_initial
# e.g.: _pickle.PicklingError: Can't pickle <class 'testapp.Clicks'>: it's not the same object as testapp.Clicks
try:
widget, render_context = _run_app(
app_state,
app_script,
pathname,
render_context=render_context,
)
if render_context is None:
assert context.container is not None
context.container.children = [widget]
except Exception:
if settings.main.use_pdb:
logger.exception("Exception, will be handled by debugger")
pdb.post_mortem()
raise

if render_context:
context.app_object = render_context
if not reload.reloader.requires_reload:
break
app_state = app_state_initial
try:
widget, render_context = _run_app(
app_state,
app_script,
pathname,
render_context=render_context,
)
if render_context is None:
assert context.container is not None
context.container.children = [widget]
except Exception:
if settings.main.use_pdb:
logger.exception("Exception, will be handled by debugger")
pdb.post_mortem()
raise

if render_context:
context.app_object = render_context

except BaseException as e:
error = ""
Expand Down
9 changes: 6 additions & 3 deletions solara/server/reload.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,17 +156,20 @@ def reload(self):
# before we did this:
# # don't reload modules like solara.server and react
# # that may cause issues (like 2 Element classes existing)
reload_modules = {k for k in set(sys.modules) - set(self.ignore_modules) if not (k.startswith("solara.server") or k.startswith("anyio"))}
# not sure why, but if we reload pandas, the integration/reload_test.py fails
reload_modules = {
k for k in set(sys.modules) - set(self.ignore_modules) if not (k.startswith("solara.server") or k.startswith("anyio") or k.startswith("pandas"))
}
# which picks up import that are done in threads etc, but it will also reload starlette, httptools etc
# which causes issues with exceptions and isinstance checks.
# reload_modules = self.watched_modules
logger.info("Reloading modules... %s", reload_modules)
# not sure if this is needed
importlib.invalidate_caches()
for mod in reload_modules:
for mod in sorted(reload_modules):
# don't reload modules like solara.server and react
# that may cause issues (like 2 Element classes existing)
logger.info("Reloading module %s", mod)
logger.debug("Reloading module %s", mod)
sys.modules.pop(mod, None)
# if all succesfull...
self.requires_reload = False
Expand Down
2 changes: 0 additions & 2 deletions solara/server/starlette.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
from starlette.staticfiles import StaticFiles

import solara
from solara.server import reload
from solara.server.threaded import ServerBase

from . import app as appmod
Expand Down Expand Up @@ -267,7 +266,6 @@ def lookup_path(self, path: str) -> typing.Tuple[str, typing.Optional[os.stat_re
def on_startup():
# TODO: configure and set max number of threads
# see https://github.com/encode/starlette/issues/1724
reload.reloader.start()
telemetry.server_start()


Expand Down
134 changes: 100 additions & 34 deletions tests/integration/reload_test.py
Original file line number Diff line number Diff line change
@@ -1,48 +1,102 @@
# import contextlib
# import logging
# from pathlib import Path
import contextlib
import logging

# import subprocess
from pathlib import Path

# import playwright.sync_api

# from solara.server import reload
# import solara.server.server

# app_path = Path(__file__).parent / "testapp.py"
# from . import conftest

# logger = logging.getLogger("solara-test.integration.reload_test")
app_path = Path(__file__).parent / "testapp.py"

logger = logging.getLogger("solara-test.integration.reload_test")

# @contextlib.contextmanager
# def append(text):
# with app_path.open() as f:
# content = f.read()
# try:
# with app_path.open("w") as f:
# f.write(content)
# f.write(text)
# yield
# finally:
# with app_path.open("w") as f:
# f.write(content)


# @contextlib.contextmanager
# def replace(path, text):
# with path.open() as f:
# content = f.read()
# try:
# with path.open("w") as f:
# f.write(text)
# yield
# finally:
# with path.open("w") as f:
# f.write(content)

HERE = Path(__file__).parent


@contextlib.contextmanager
def append(text):
with app_path.open() as f:
content = f.read()
try:
with app_path.open("w") as f:
f.write(content)
f.write(text)
yield
finally:
with app_path.open("w") as f:
f.write(content)


@contextlib.contextmanager
def replace(path, text):
with path.open() as f:
content = f.read()
try:
with path.open("w") as f:
f.write(text)
yield
finally:
with path.open("w") as f:
f.write(content)


# button_code = """
# @solara.component
# def ButtonClick():
# clicks, set_clicks = solara.use_state(Clicks(0))
# return rw.Button(description=f"!!! {clicks.value} times", on_click=lambda: set_clicks(Clicks(clicks.value + 1)))


# app = ButtonClick()
# """


# def test_reload_with_pickle(page_session: playwright.sync_api.Page):
# port = conftest.TEST_PORT + 1000
# conftest.TEST_PORT += 1
# args = ["solara", "run", f"--port={port}", "--no-open", "tests.integration.testapp:app", "--log-level=debug"]
# popen = subprocess.Popen(args, shell=False, stdout=subprocess.PIPE, stderr=subprocess.PIPE, stdin=subprocess.PIPE)
# host = "localhost"
# try:
# solara.server.server.wait_ready(f"http://{host}:{port}", timeout=10)
# page_session.goto(f"http://localhost:{port}")
# page_session.locator("text=Clicked 0 times").click(timeout=5000)
# page_session.locator("text=Clicked 1 times").click(timeout=5000)
# page_session.locator("text=Clicked 2 times").wait_for(timeout=5000)
# page_session.wait_for_timeout(1000)
# with append(button_code):
# page_session.locator("text=!!! 0 times").click(timeout=5000)
# page_session.locator("text=!!! 1 times").click(timeout=5000)
# popen.kill()
# except Exception as e:
# try:
# popen.kill()
# except: # noqa
# pass
# outs, errs = popen.communicate(timeout=5)
# if errs:
# print("STDERR:") # noqa
# print(errs.decode("utf-8")) # noqa
# if outs:
# print("STDOUT:") # noqa
# print(outs.decode("utf-8")) # noqa
# if errs:
# raise ValueError("Expected no errors in solara server output") from e
# raise


# the following tests are flakey on CI

# def test_reload_syntax_error(page_session: playwright.sync_api.Page, solara_server, solara_app, extra_include_path):
# with extra_include_path(app_path.parent), solara_app("testapp:ButtonClick"):
# # use as module, otherwise pickle wil not work
# page_session.goto(solara_server.base_url)
# assert page_session.title() == "Hello from Solara ☀️"
# assert page_session.title() == "Solara ☀️"
# page_session.locator("text=Clicked 0 times").click()
# page_session.locator("text=Clicked 1 times").click()

Expand All @@ -55,12 +109,25 @@
# page_session.locator("text=Clicked 3 times").wait_for()


# def test_reload_change(page_session: playwright.sync_api.Page, solara_server, solara_app, extra_include_path):
# with extra_include_path(app_path.parent), solara_app("testapp:app"):
# logger.info("test_reload_many:run app")
# page_session.goto(solara_server.base_url)
# # assert page_session.title() == "Solara ☀️"
# page_session.locator("text=Clicked 0 times").click()
# page_session.locator("text=Clicked 1 times").click()
# with append(button_code):
# reload.reloader.reload_event_next.wait()
# # page_session.locator("text=Clicked 2 times").click()
# # page_session.locator("text=SyntaxError").wait_for()
# page_session.locator("text=!!! 0 times").click()


# def test_reload_many(page_session: playwright.sync_api.Page, solara_server, solara_app, extra_include_path):
# with extra_include_path(app_path.parent), solara_app("testapp:app"):
# logger.info("test_reload_many:run app")
# # use as module, otherwise pickle wil not work
# page_session.goto(solara_server.base_url)
# assert page_session.title() == "Hello from Solara ☀️"
# page_session.locator("text=Clicked 0 times").click()
# page_session.locator("text=Clicked 1 times").click()

Expand All @@ -80,7 +147,6 @@
# def test_reload_vue(page_session: playwright.sync_api.Page, solara_server, solara_app, extra_include_path):
# with extra_include_path(app_path.parent), solara_app("testapp:VueTestApp"):
# page_session.goto(solara_server.base_url)
# assert page_session.title() == "Hello from Solara ☀️"
# page_session.locator("text=foobar").wait_for()

# vuecode = """
Expand Down
3 changes: 2 additions & 1 deletion tests/integration/testapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
import os

import ipyvue
import solara
import traitlets

import solara
from solara.alias import rw


Expand Down
Loading

0 comments on commit 478d19c

Please sign in to comment.