Skip to content

Commit

Permalink
Make autoreload robust to syntax errors and empty apps (#7028)
Browse files Browse the repository at this point in the history
  • Loading branch information
philippjfr authored Jul 27, 2024
1 parent c6d0ff3 commit 78982fb
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 15 deletions.
17 changes: 12 additions & 5 deletions panel/command/serve.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
ways.
"""

import argparse
import ast
import base64
import logging
Expand Down Expand Up @@ -279,10 +280,14 @@ def customize_applications(self, args, applications):
applications['/'] = applications[f'/{index}']
return super().customize_applications(args, applications)

def warm_applications(self, applications, reuse_sessions):
def warm_applications(self, applications, reuse_sessions, error=True):
from ..io.session import generate_session
for path, app in applications.items():
session = generate_session(app)
try:
session = generate_session(app)
except Exception as e:
if error:
raise e
with set_curdoc(session.document):
if config.session_key_func:
reuse_sessions = False
Expand Down Expand Up @@ -345,7 +350,6 @@ def customize_kwargs(self, args, server_kwargs):
elif args.rest_provider is not None:
raise ValueError(f"rest-provider {args.rest_provider!r} not recognized.")

config.autoreload = args.autoreload
config.global_loading_spinner = args.global_loading_spinner
config.reuse_sessions = args.reuse_sessions

Expand All @@ -371,7 +375,7 @@ def customize_kwargs(self, args, server_kwargs):
if args.autoreload:
with record_modules(list(applications.values())):
self.warm_applications(
applications, args.reuse_sessions
applications, args.reuse_sessions, error=False
)
else:
self.warm_applications(applications, args.reuse_sessions)
Expand Down Expand Up @@ -638,7 +642,10 @@ def customize_kwargs(self, args, server_kwargs):

return kwargs

def invoke(self, args):
def invoke(self, args: argparse.Namespace):
# Autoreload must be enabled before the application(s) are executed
# to avoid erroring out
config.autoreload = args.autoreload
# Empty layout are valid and the Bokeh warning is silenced as usually
# not relevant to Panel users.
silence(EMPTY_LAYOUT, True)
Expand Down
2 changes: 1 addition & 1 deletion panel/io/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ def build_single_handler_application(path, argv=None):
else:
raise ValueError(f"Path for Bokeh server application does not exist: {path}")

if handler.failed:
if handler.failed and not config.autoreload:
raise RuntimeError(f"Error loading {path}:\n\n{handler.error}\n{handler.error_detail} ")

application = Application(handler)
Expand Down
43 changes: 34 additions & 9 deletions panel/io/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ def autoreload_handle_exception(handler, module, e):
alert_type='danger', margin=5, sizing_mode='stretch_width'
).servable()

def run_app(handler, module, doc, post_run=None):
def run_app(handler, module, doc, post_run=None, allow_empty=False):
try:
old_doc = curdoc()
except RuntimeError:
Expand All @@ -266,9 +266,25 @@ def post_check():
with patch_curdoc(doc):
with profile_ctx(config.profiler) as sessions:
with record_modules(handler=handler):
handler._runner.run(module, post_check)
if post_run:
post_run()
runner = handler._runner
if runner.error:
from ..pane import Alert
Alert(
f'<b>{runner.error}</b>\n<pre style="overflow-y: auto">{runner.error_detail}</pre>',
alert_type='danger', margin=5, sizing_mode='stretch_width'
).servable()
else:
handler._runner.run(module, post_check)
if post_run:
post_run()
if not doc.roots and not allow_empty and config.autoreload:
from ..pane import Alert
Alert(
('<b>Application did not publish any contents</b>\n\n<span>'
'Ensure you have marked items as servable or added models to '
'the bokeh document manually.'),
alert_type='danger', margin=5, sizing_mode='stretch_width'
).servable()
finally:
if config.profiler:
try:
Expand Down Expand Up @@ -422,6 +438,13 @@ def __init__(self, *, source: str, filename: PathLike, argv: list[str] = [], pac
for f in PanelCodeHandler._io_functions:
self._loggers[f] = self._make_io_logger(f)

def url_path(self) -> str | None:
if self.failed and not config.autoreload:
return None

# TODO should fix invalid URL characters
return '/' + os.path.splitext(os.path.basename(self._runner.path))[0]

def modify_document(self, doc: 'Document'):
if config.autoreload:
path = self._runner.path
Expand All @@ -433,14 +456,15 @@ def modify_document(self, doc: 'Document'):

# If no module was returned it means the code runner has some permanent
# unfixable problem, e.g. the configured source code has a syntax error
if module is None:
if module is None and not config.autoreload:
return

# One reason modules are stored is to prevent the module from being gc'd
# before the document is. A symptom of a gc'd module is that its globals
# become None. Additionally stored modules are used to provide correct
# paths to custom models resolver.
doc.modules.add(module)
if module is not None:
doc.modules.add(module)

run_app(self, module, doc)

Expand Down Expand Up @@ -649,14 +673,15 @@ def modify_document(self, doc: Document) -> None:

# If no module was returned it means the code runner has some permanent
# unfixable problem, e.g. the configured source code has a syntax error
if module is None:
if module is None and not config.autoreload:
return

# One reason modules are stored is to prevent the module from being gc'd
# before the document is. A symptom of a gc'd module is that its globals
# become None. Additionally stored modules are used to provide correct
# paths to custom models resolver.
doc.modules.add(module)
if module is not None:
doc.modules.add(module)

def post_run():
if not (doc.roots or doc in state._templates or self._runner.error):
Expand All @@ -665,7 +690,7 @@ def post_run():

with _patch_ipython_display():
with set_env_vars(MPLBACKEND='agg'):
run_app(self, module, doc, post_run)
run_app(self, module, doc, post_run, allow_empty=True)

def _update_position_metadata(self, event):
"""
Expand Down
27 changes: 27 additions & 0 deletions panel/tests/ui/io/test_reload.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,33 @@ def test_reload_app_with_error(page, autoreload, py_file):

expect(page.locator('.alert')).to_have_count(1)

def test_reload_app_with_syntax_error(page, autoreload, py_file):
py_file.write("import panel as pn; pn.panel('foo').servable();")
py_file.close()

path = pathlib.Path(py_file.name)

autoreload(path)
serve_component(page, path)

expect(page.locator('.markdown')).to_have_text('foo')

with open(py_file.name, 'w') as f:
f.write("foo?bar")
os.fsync(f)

expect(page.locator('.alert')).to_have_count(1)

def test_load_app_with_no_content(page, autoreload, py_file):
py_file.write("import panel as pn; pn.panel('foo')")
py_file.close()

path = pathlib.Path(py_file.name)

serve_component(page, path)

expect(page.locator('.alert')).to_have_count(1)

@pytest.mark.flaky(reruns=3, reason="Writing files can sometimes be unpredictable")
def test_reload_app_on_local_module_change(page, autoreload, py_files):
py_file, module = py_files
Expand Down

0 comments on commit 78982fb

Please sign in to comment.