From d40b53748dfdeab9e5282727fb7340552f9d14dd Mon Sep 17 00:00:00 2001 From: Brendon Smith Date: Sun, 20 Nov 2022 02:43:10 -0500 Subject: [PATCH 1/2] Fix watchfiles file filtering The module `uvicorn.supervisors.watchfilesreload` implements watchfiles file watching. The module includes a `FileFilter` class. Currently, the `FileFilter` class is instantiated when initializing `class WatchFilesReload`, but not passed to `watchfiles.watch()`. Instead, `watchfiles.watch(watch_filter=None)` is used. As a result, `watchfiles` does not filter out the expected changes. It detects changes to `.mypy_cache` and `.pyc` files that should be ignored. This commit will: - Update the arguments to `FileFilter.__call__` to match those expected by `watchfiles.watch()`. - Pass the `FileFilter` instance to `watchfiles.watch()`. `watchfiles` will correctly filter `.mypy_cache`/`.pyc` file changes. https://watchfiles.helpmanual.io/api/watch/ --- uvicorn/supervisors/watchfilesreload.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/uvicorn/supervisors/watchfilesreload.py b/uvicorn/supervisors/watchfilesreload.py index 9ba10ce9e..7da361d3c 100644 --- a/uvicorn/supervisors/watchfilesreload.py +++ b/uvicorn/supervisors/watchfilesreload.py @@ -2,7 +2,7 @@ from socket import socket from typing import Callable, List, Optional -from watchfiles import watch +from watchfiles import Change, watch from uvicorn.config import Config from uvicorn.supervisors.basereload import BaseReload @@ -40,7 +40,8 @@ def __init__(self, config: Config): self.excludes.append(e) self.excludes = list(set(self.excludes)) - def __call__(self, path: Path) -> bool: + def __call__(self, change: Change, path_string: str) -> bool: + path = Path(path_string) for include_pattern in self.includes: if path.match(include_pattern): for exclude_dir in self.exclude_dirs: @@ -74,7 +75,7 @@ def __init__( self.watch_filter = FileFilter(config) self.watcher = watch( *self.reload_dirs, - watch_filter=None, + watch_filter=self.watch_filter, stop_event=self.should_exit, # using yield_on_timeout here mostly to make sure tests don't # hang forever, won't affect the class's behavior @@ -84,6 +85,6 @@ def __init__( def should_restart(self) -> Optional[List[Path]]: changes = next(self.watcher) if changes: - unique_paths = {Path(c[1]) for c in changes} - return [p for p in unique_paths if self.watch_filter(p)] + unique_paths = {Path(c[1]) for c in changes if self.watch_filter(*c)} + return [p for p in unique_paths] return None From 41a043566e494d60f5d81b65aaa9296113b30687 Mon Sep 17 00:00:00 2001 From: Brendon Smith Date: Mon, 21 Nov 2022 19:41:05 -0500 Subject: [PATCH 2/2] Remove duplicate call to `FileFilter.__call__` The `uvicorn.supervisors.watchfilesreload.FileFilter.__call__` method was being called twice when reloading with watchfiles: 1. `watchfiles.main._prep_changes` 2. `uvicorn.supervisors.watchfilesreload.WatchFilesReload.should_restart`. This commit will drop the duplicate call from `should_restart`, and add a test that patches `FileFilter.__call__` and tracks the call count. https://github.com/encode/uvicorn/pull/1770#issuecomment-1321117094 --- tests/supervisors/test_reload.py | 20 ++++++++++++++++++++ uvicorn/supervisors/watchfilesreload.py | 2 +- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/tests/supervisors/test_reload.py b/tests/supervisors/test_reload.py index 4f3190117..cc5cf7d4f 100644 --- a/tests/supervisors/test_reload.py +++ b/tests/supervisors/test_reload.py @@ -279,6 +279,26 @@ def test_watchfiles_no_changes(self) -> None: reloader.shutdown() + @pytest.mark.skipif(WatchFilesReload is None, reason="watchfiles not available") + @pytest.mark.parametrize("reloader_class", [WatchFilesReload]) + def test_watchfiles_should_only_call_watch_filter_once_per_reload( + self, mocker, touch_soon + ): + mock_call = mocker.patch( + "uvicorn.supervisors.watchfilesreload.FileFilter.__call__" + ) + file = self.reload_path / "main.py" + + with as_cwd(self.reload_path): + config = Config(app="tests.test_config:asgi_app", reload=True) + reloader = self._setup_reloader(config) + + self._reload_tester(touch_soon, reloader, file) + + mock_call.assert_called_once() + + reloader.shutdown() + @pytest.mark.parametrize("reloader_class", [WatchGodReload]) def test_should_detect_new_reload_dirs( self, touch_soon, caplog: pytest.LogCaptureFixture, tmp_path: Path diff --git a/uvicorn/supervisors/watchfilesreload.py b/uvicorn/supervisors/watchfilesreload.py index 7da361d3c..796bd5048 100644 --- a/uvicorn/supervisors/watchfilesreload.py +++ b/uvicorn/supervisors/watchfilesreload.py @@ -85,6 +85,6 @@ def __init__( def should_restart(self) -> Optional[List[Path]]: changes = next(self.watcher) if changes: - unique_paths = {Path(c[1]) for c in changes if self.watch_filter(*c)} + unique_paths = {Path(c[1]) for c in changes} return [p for p in unique_paths] return None