From 78982fb9d3d723f9e61df28075cf4b96f9470815 Mon Sep 17 00:00:00 2001 From: Philipp Rudiger Date: Sat, 27 Jul 2024 15:00:14 +0200 Subject: [PATCH] Make autoreload robust to syntax errors and empty apps (#7028) --- panel/command/serve.py | 17 +++++++++---- panel/io/application.py | 2 +- panel/io/handlers.py | 43 +++++++++++++++++++++++++------- panel/tests/ui/io/test_reload.py | 27 ++++++++++++++++++++ 4 files changed, 74 insertions(+), 15 deletions(-) diff --git a/panel/command/serve.py b/panel/command/serve.py index fe21ecbec3..cbd19b7eb8 100644 --- a/panel/command/serve.py +++ b/panel/command/serve.py @@ -3,6 +3,7 @@ ways. """ +import argparse import ast import base64 import logging @@ -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 @@ -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 @@ -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) @@ -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) diff --git a/panel/io/application.py b/panel/io/application.py index ee8906ad56..ae3062c98c 100644 --- a/panel/io/application.py +++ b/panel/io/application.py @@ -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) diff --git a/panel/io/handlers.py b/panel/io/handlers.py index fec9ab2651..e30c144205 100644 --- a/panel/io/handlers.py +++ b/panel/io/handlers.py @@ -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: @@ -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'{runner.error}\n
{runner.error_detail}
', + 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( + ('Application did not publish any contents\n\n' + '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: @@ -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 @@ -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) @@ -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): @@ -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): """ diff --git a/panel/tests/ui/io/test_reload.py b/panel/tests/ui/io/test_reload.py index 4725e05a04..cc5b582375 100644 --- a/panel/tests/ui/io/test_reload.py +++ b/panel/tests/ui/io/test_reload.py @@ -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