From 0482e431a5a422aca78299de737f8e958ab72cbb Mon Sep 17 00:00:00 2001 From: Philipp Rudiger Date: Fri, 12 Jan 2024 15:49:51 +0100 Subject: [PATCH] Use watchfiles library for detecting changed files (#5894) --- doc/api/config.md | 2 +- doc/getting_started/core_concepts.md | 4 + doc/how_to/server/commandline.md | 5 +- panel/io/reload.py | 153 ++++++++++++++++++--------- panel/io/server.py | 16 ++- panel/io/state.py | 1 + panel/tests/conftest.py | 13 ++- panel/tests/io/test_reload.py | 31 +++--- panel/tests/util.py | 4 +- 9 files changed, 157 insertions(+), 72 deletions(-) diff --git a/doc/api/config.md b/doc/api/config.md index 4bab8cad32..ed99432620 100644 --- a/doc/api/config.md +++ b/doc/api/config.md @@ -70,7 +70,7 @@ Default: None | Type: Callable ### `autoreload` -Whether to autoreload server when script changes. +Whether to autoreload the page whenever the script or one of its dependencies changes. Default: False | Type: Boolean diff --git a/doc/getting_started/core_concepts.md b/doc/getting_started/core_concepts.md index dfd1431ebd..7d38f1faa1 100644 --- a/doc/getting_started/core_concepts.md +++ b/doc/getting_started/core_concepts.md @@ -41,6 +41,10 @@ Once you run that command Panel will launch a server that will serve your app, o +```{note} +We recommend installing `watchfiles`, which will provide a significantly better user experience when using `--autoreload`. +``` + > Checkout [How-to > Prepare to develop](../how_to/prepare_to_develop.md) for more guidance on each of the development environment options. ## Control flow diff --git a/doc/how_to/server/commandline.md b/doc/how_to/server/commandline.md index 5b6666aa10..1b73cab843 100644 --- a/doc/how_to/server/commandline.md +++ b/doc/how_to/server/commandline.md @@ -14,8 +14,11 @@ or even serve a number of apps at once: For development it can be particularly helpful to use the ``--autoreload`` option to `panel serve` as that will automatically reload the page whenever the application code or any of its imports change. -The ``panel serve`` command has the following options: +```{note} +We recommend installing `watchfiles`, which will provide a significantly better user experience when using `--autoreload`. +``` +The ``panel serve`` command has the following options: ``` console positional arguments: diff --git a/panel/io/reload.py b/panel/io/reload.py index 0a2daacd2b..3d4dc3f8cb 100644 --- a/panel/io/reload.py +++ b/panel/io/reload.py @@ -1,18 +1,36 @@ +import asyncio import fnmatch +import logging import os import sys import types +import warnings from contextlib import contextmanager -from functools import partial + +try: + from watchfiles import awatch +except Exception: + async def awatch(*files, stop_event=None): + modify_times = {} + stop_event = stop_event if stop_event else asyncio.Event() + while not stop_event.is_set(): + changes = set() + for path in files: + change = _check_file(path, modify_times) + if change: + changes.add((change, path)) + if changes: + yield changes + await asyncio.sleep(0.5) from ..util import fullpath -from .callbacks import PeriodicCallback from .state import state +_reload_logger = logging.getLogger('panel.io.reload') + _watched_files = set() _modules = set() -_callbacks = {} # List of paths to ignore DEFAULT_FOLDER_DENYLIST = [ @@ -65,16 +83,52 @@ def file_is_in_folder_glob(filepath, folderpath_glob): file_dir = os.path.dirname(filepath) + "/" return fnmatch.fnmatch(file_dir, folderpath_glob) -def autoreload_watcher(): +async def async_file_watcher(stop_event=None): + files = list(_watched_files) + modules = {} + for module_name in _modules: + # Some modules play games with sys.modules (e.g. email/__init__.py + # in the standard library), and occasionally this can cause strange + # failures in getattr. Just ignore anything that's not an ordinary + # module. + if module_name not in sys.modules: + continue + module = sys.modules[module_name] + if not isinstance(module, types.ModuleType): + continue + path = getattr(module, "__file__", None) + if not path: + continue + if path.endswith((".pyc", ".pyo")): + path = path[:-1] + modules[path] = module_name + files.append(path) + + async for changes in awatch(*files, stop_event=stop_event): + for _, path in changes: + if path in modules: + module = modules[path] + if module in sys.modules: + del sys.modules[module] + _reload(changes) + +async def setup_autoreload_watcher(stop_event=None): """ Installs a periodic callback which checks for changes in watched files and sys.modules. """ - if not state.curdoc or not state.curdoc.session_context.server_context: - return - cb = partial(_reload_on_update, {}) - _callbacks[state.curdoc] = pcb = PeriodicCallback(callback=cb, background=True) - pcb.start() + try: + import watchfiles # noqa + except Exception: + warnings.warn( + '--autoreload functionality now depends on the watchfiles ' + 'library. In future versions autoreload will not work without ' + 'watchfiles being installed. Since it provides a much better ' + 'user experience consider installing it today.', FutureWarning, + stacklevel=0 + ) + _reload_logger.debug('Setting up global autoreload watcher.') + await async_file_watcher(stop_event=stop_event) def watch(filename): """ @@ -117,48 +171,49 @@ def record_modules(): except Exception: continue -def _reload(module=None): - if module is not None: - for module in _modules: - if module in sys.modules: - del sys.modules[module] - for cb in _callbacks.values(): - cb.stop() - _callbacks.clear() - if state.location is not None: - # In case session has been cleaned up - state.location.reload = True - for loc in state._locations.values(): - loc.reload = True - -def _check_file(modify_times, path, module=None): +def _reload(changes): + _reload_logger.debug('Changes detected by autoreload watcher, reloading sessions.') + for doc, loc in state._locations.items(): + if not doc.session_context: + continue + elif state._loaded.get(doc): + loc.reload = True + continue + def reload_session(event, loc=loc): + loc.reload = True + doc.on_event('document_ready', reload_session) + +def _check_file(path, modify_times): + """ + Checks if a file was modified or deleted and then returns a code, + modeled after watchfiles, indicating the type of change: + + - 0: No change + - 2: File modified + - 3: File deleted + + Arguments + --------- + path: str | os.PathLike + Path of the file to check for modification + modify_times: dict[str, int] + Dictionary of modification times for different paths. + + Returns + ------- + Status code indicating type of change. + """ + last_modified = modify_times.get(path) try: modified = os.stat(path).st_mtime + except FileNotFoundError: + if last_modified: + return 3 except Exception: - return - if path not in modify_times: + return 0 + if last_modified is None: modify_times[path] = modified - return - if modify_times[path] != modified: - _reload(module) + return 0 + elif last_modified != modified: modify_times[path] = modified - -def _reload_on_update(modify_times): - for module_name in _modules: - # Some modules play games with sys.modules (e.g. email/__init__.py - # in the standard library), and occasionally this can cause strange - # failures in getattr. Just ignore anything that's not an ordinary - # module. - if module_name not in sys.modules: - continue - module = sys.modules[module_name] - if not isinstance(module, types.ModuleType): - continue - path = getattr(module, "__file__", None) - if not path: - continue - if path.endswith((".pyc", ".pyo")): - path = path[:-1] - _check_file(modify_times, path, module_name) - for path in _watched_files: - _check_file(modify_times, path) + return 2 diff --git a/panel/io/server.py b/panel/io/server.py index dc9f01b9a5..ad3e1496bd 100644 --- a/panel/io/server.py +++ b/panel/io/server.py @@ -81,7 +81,6 @@ ) from .markdown import build_single_handler_application from .profile import profile_ctx -from .reload import autoreload_watcher from .resources import ( BASE_TEMPLATE, CDN_DIST, COMPONENT_PATH, ERROR_TEMPLATE, LOCAL_DIST, Resources, _env, bundle_resources, patch_model_css, resolve_custom_path, @@ -378,6 +377,7 @@ class Server(BokehServer): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) + self._autoreload_stop_event = None if state._admin_context: state._admin_context._loop = self._loop @@ -385,8 +385,18 @@ def start(self) -> None: super().start() if state._admin_context: self._loop.add_callback(state._admin_context.run_load_hook) + if config.autoreload: + from .reload import setup_autoreload_watcher + self._autoreload_stop_event = stop_event = asyncio.Event() + self._loop.add_callback(setup_autoreload_watcher, stop_event=stop_event) def stop(self, wait: bool = True) -> None: + if self._autoreload_stop_event: + self._autoreload_stop_event.set() + # For the stop event to be processed we have to restart + # the IOLoop briefly, ensuring an orderly cleanup + sleep = asyncio.sleep(0.1) + self._loop.asyncio_loop.run_until_complete(sleep) super().stop(wait=wait) if state._admin_context: state._admin_context.run_unload_hook() @@ -802,10 +812,6 @@ def modify_document(self, doc: 'Document'): old_doc = None bk_set_curdoc(doc) - if config.autoreload: - set_curdoc(doc) - state.onload(autoreload_watcher) - sessions = [] try: diff --git a/panel/io/state.py b/panel/io/state.py index 6336b10751..0c4d527610 100644 --- a/panel/io/state.py +++ b/panel/io/state.py @@ -757,6 +757,7 @@ def reset(self): """ self.kill_all_servers() self._indicators.clear() + self._location = None self._locations.clear() self._templates.clear() self._views.clear() diff --git a/panel/tests/conftest.py b/panel/tests/conftest.py index 3cf51198dd..e4c63561b2 100644 --- a/panel/tests/conftest.py +++ b/panel/tests/conftest.py @@ -1,6 +1,7 @@ """ A module containing testing utilities and fixtures. """ +import asyncio import atexit import os import pathlib @@ -24,6 +25,7 @@ from panel import config, serve from panel.config import panel_extension +from panel.io.reload import _modules, _watched_files from panel.io.state import set_curdoc, state from panel.pane import HTML, Markdown @@ -148,7 +150,6 @@ def context(context): def document(): return Document() - @pytest.fixture def server_document(): doc = Document() @@ -156,11 +157,19 @@ def server_document(): doc._session_context = lambda: session_context with set_curdoc(doc): yield doc + doc._session_context = None @pytest.fixture def comm(): return Comm() +@pytest.fixture +def stop_event(): + event = asyncio.Event() + try: + yield event + finally: + event.set() @pytest.fixture def port(): @@ -332,6 +341,8 @@ def server_cleanup(): yield finally: state.reset() + _watched_files.clear() + _modules.clear() @pytest.fixture(autouse=True) def cache_cleanup(): diff --git a/panel/tests/io/test_reload.py b/panel/tests/io/test_reload.py index a9f7daf8a7..0c69cd406d 100644 --- a/panel/tests/io/test_reload.py +++ b/panel/tests/io/test_reload.py @@ -1,13 +1,16 @@ +import asyncio import os +import tempfile import pytest from panel.io.location import Location from panel.io.reload import ( - _check_file, _modules, _reload_on_update, _watched_files, in_denylist, + _check_file, _modules, _watched_files, async_file_watcher, in_denylist, record_modules, watch, ) from panel.io.state import state +from panel.tests.util import async_wait_until def test_record_modules_not_stdlib(): @@ -18,7 +21,7 @@ def test_record_modules_not_stdlib(): def test_check_file(): modify_times = {} - _check_file(modify_times, __file__) + _check_file(__file__, modify_times) assert modify_times[__file__] == os.stat(__file__).st_mtime def test_file_in_denylist(): @@ -37,15 +40,17 @@ def test_watch(): _watched_files.clear() @pytest.mark.flaky(reruns=3) -def test_reload_on_update(): +async def test_reload_on_update(server_document, stop_event): location = Location() - state._location = location - filepath = os.path.abspath(__file__) - watch(filepath) - modify_times = {filepath: os.stat(__file__).st_mtime-1} - _reload_on_update(modify_times) - assert location.reload - - # Cleanup - _watched_files.clear() - state._location = None + state._locations[server_document] = location + state._loaded[server_document] = True + with tempfile.NamedTemporaryFile() as temp: + temp.write(b'Foo') + temp.flush() + watch(temp.name) + task = asyncio.create_task(async_file_watcher(stop_event)) + await asyncio.sleep(0.51) + temp.write(b'Bar') + temp.flush() + await async_wait_until(lambda: location.reload) + del task diff --git a/panel/tests/util.py b/panel/tests/util.py index cf0c83edac..bb4c455ec2 100644 --- a/panel/tests/util.py +++ b/panel/tests/util.py @@ -232,7 +232,7 @@ def timed_out(): elapsed_ms = elapsed * 1000 return elapsed_ms > timeout - timeout_msg = f"wait_until timed out in {timeout} milliseconds" + timeout_msg = f"async_wait_until timed out in {timeout} milliseconds" while True: try: @@ -245,7 +245,7 @@ def timed_out(): else: if result not in (None, True, False): raise ValueError( - "`wait_until` callback must return None, True, or " + "`async_wait_until` callback must return None, True, or " f"False, returned {result!r}" ) # None is returned when the function has an assert