From 965ede7b047fefb46abb4144051b181dda150334 Mon Sep 17 00:00:00 2001 From: Amin Ghadersohi Date: Tue, 12 May 2026 21:19:57 -0700 Subject: [PATCH 1/7] fix(extensions): make LOCAL_EXTENSIONS hot reload reliable in Docker The local-extensions watcher previously surfaced spurious reloads under Docker on macOS (VirtioFS / osxfs generates inotify events on every read, not just writes), and rebuilds that wrote many files at once produced one restart per file. It also touched superset/__init__.py to trigger Flask's reloader, which then meant any Python read of that file also looked like a change and recursed. Changes: - Watcher only acts on FileCreated/Modified/Moved events, verifies the file content actually changed via SHA-256, and debounces to one trigger per second. - Use a dedicated sentinel file (superset/extensions/.reload_trigger) that Python never reads, registered with Flask via --extra-files. The watcher ensures the sentinel exists on startup. - Bootstrap excludes superset/__init__.py from the file watcher so the old trigger path can't reintroduce the loop. - docker-compose mounts ./local_extensions:/app/local_extensions so the bind path matches LOCAL_EXTENSIONS in the dev superset_config. - Extension content endpoint and FRONTEND_REGEX accept nested paths inside frontend/dist so extensions can serve worker / WASM / chunk subfolders. Co-Authored-By: Claude Opus 4.7 --- docker-compose.yml | 1 + docker/docker-bootstrap.sh | 4 +- superset/extensions/api.py | 2 +- .../extensions/local_extensions_watcher.py | 102 ++++++++++++++++-- superset/extensions/utils.py | 7 +- 5 files changed, 104 insertions(+), 12 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 6d1e2bc3f059..f333f275aa7f 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -34,6 +34,7 @@ x-superset-volumes: &superset-volumes - superset_home:/app/superset_home - ./tests:/app/tests - superset_data:/app/data + - ./local_extensions:/app/local_extensions x-common-build: &common-build context: . target: ${SUPERSET_BUILD_TARGET:-dev} # can use `dev` (default) or `lean` diff --git a/docker/docker-bootstrap.sh b/docker/docker-bootstrap.sh index 35871fc42086..156471510053 100755 --- a/docker/docker-bootstrap.sh +++ b/docker/docker-bootstrap.sh @@ -96,7 +96,9 @@ case "${1}" in echo " 🔒 Werkzeug debugger disabled (set SUPERSET_DEBUG_ENABLED=true to enable)" fi - flask run -p $PORT --reload $DEBUGGER_FLAG --host=0.0.0.0 --exclude-patterns "*/node_modules/*:*/.venv/*:*/build/*:*/__pycache__/*:*/superset-frontend/*" + flask run -p $PORT --reload $DEBUGGER_FLAG --host=0.0.0.0 \ + --extra-files "/app/superset/extensions/.reload_trigger" \ + --exclude-patterns "*/node_modules/*:*/.venv/*:*/build/*:*/__pycache__/*:*/superset-frontend/*:*/superset/__init__.py" ;; app-gunicorn) echo "Starting web app..." diff --git a/superset/extensions/api.py b/superset/extensions/api.py index b1b5734979ee..8ed3129d34c6 100644 --- a/superset/extensions/api.py +++ b/superset/extensions/api.py @@ -169,7 +169,7 @@ def get(self, publisher: str, name: str, **kwargs: Any) -> Response: @protect() @safe - @expose("///", methods=("GET",)) + @expose("///", methods=("GET",)) def content(self, publisher: str, name: str, file: str) -> Response: """Get a frontend chunk of an extension. --- diff --git a/superset/extensions/local_extensions_watcher.py b/superset/extensions/local_extensions_watcher.py index 6233f91fe5cf..4a6e3ad8f66c 100644 --- a/superset/extensions/local_extensions_watcher.py +++ b/superset/extensions/local_extensions_watcher.py @@ -29,37 +29,113 @@ logger = logging.getLogger(__name__) +# Sentinel file Flask watches via --extra-files. Touching it on a real change +# triggers a server reload without depending on cwd or the location of any +# Python source file. +RELOAD_TRIGGER = Path(__file__).resolve().parent / ".reload_trigger" + # Guard to prevent multiple initializations _watcher_initialized = False _watcher_lock = threading.Lock() -def _get_file_handler_class() -> Any: +def _get_file_handler_class() -> Any: # noqa: C901 """Get the file handler class, importing watchdog only when needed.""" try: - from watchdog.events import FileSystemEventHandler + import hashlib + + from watchdog.events import ( + FileCreatedEvent, + FileModifiedEvent, + FileMovedEvent, + FileSystemEventHandler, + ) class LocalExtensionFileHandler(FileSystemEventHandler): - """Custom file system event handler for LOCAL_EXTENSIONS directories.""" + """Custom file system event handler for LOCAL_EXTENSIONS directories. + + Only reacts to genuine content changes (create / modify / move) in the + dist directory, verified by comparing a SHA-256 of the file's content. + This avoids the Docker VirtioFS / osxfs problem where reading a file + generates inotify events that watchdog surfaces as modifications. + """ + + def __init__(self) -> None: + super().__init__() + # sha256 of last-seen content, keyed by absolute path + self._file_hashes: dict[str, str] = {} + # Deduplicate: only trigger once per second across all files + self._last_trigger: float = 0.0 + self._lock = threading.Lock() + + # ── helpers ────────────────────────────────────────────────────── + + @staticmethod + def _sha256(path: str) -> str | None: + try: + with open(path, "rb") as fh: + return hashlib.sha256(fh.read()).hexdigest() + except OSError: + return None + + def _content_changed(self, path: str) -> bool: + """Return True only when the file's content differs from last seen. + + The first time a path is observed its hash is stored as the baseline + and False is returned — that event is a 'first-seen', not a change. + Only a subsequent event where the digest differs from the baseline + is treated as a genuine content change. + """ + digest = self._sha256(path) + if digest is None: + return False + old_digest = self._file_hashes.get(path) + self._file_hashes[path] = digest + if old_digest is None: + # First observation — record baseline, do not trigger restart. + return False + return old_digest != digest + + # ── event handler ───────────────────────────────────────────────── def on_any_event(self, event: Any) -> None: - """Handle any file system event in the watched directories.""" + """Handle file system events in the watched directories.""" if event.is_directory: return - # Only trigger on changes to files in `dist` directory + # Only react to true write events; skip access / close / open etc. + if not isinstance( + event, (FileCreatedEvent, FileModifiedEvent, FileMovedEvent) + ): + return + + # Only care about files inside a `dist` directory src = getattr(event, "src_path", None) if not isinstance(src, str) or "dist" not in Path(src).parts: return + # Verify the file content actually changed to ignore spurious + # inotify events generated by Docker bind-mount reads. + if not self._content_changed(src): + return + + # Debounce: one restart per second max, regardless of how many + # files webpack writes simultaneously. + now = time.monotonic() + with self._lock: + if now - self._last_trigger < 1.0: + return + self._last_trigger = now + logger.info( "File change detected in LOCAL_EXTENSIONS: %s", event.src_path ) - # Touch superset/__init__.py to trigger Flask's file watcher - superset_init = Path("superset/__init__.py") - logger.info("Triggering restart by touching %s", superset_init) - os.utime(superset_init, (time.time(), time.time())) + # Touch the dedicated reload-trigger sentinel file. + # Flask watches this via --extra-files; it is never read by Python + # so Docker VirtioFS will not generate spurious inotify events on it. + logger.info("Triggering restart by touching %s", RELOAD_TRIGGER) + os.utime(RELOAD_TRIGGER, (time.time(), time.time())) return LocalExtensionFileHandler except ImportError: @@ -130,6 +206,14 @@ def setup_local_extensions_watcher(app: Flask) -> None: # noqa: C901 if not watch_dirs: return + # Ensure the sentinel exists so os.utime() and Flask's --extra-files watcher + # both have a real path to operate on. + try: + RELOAD_TRIGGER.touch(exist_ok=True) + except OSError as e: + logger.warning("Could not create reload trigger %s: %s", RELOAD_TRIGGER, e) + return + try: from watchdog.observers import Observer diff --git a/superset/extensions/utils.py b/superset/extensions/utils.py index 4455560696cd..d9418a43c4b5 100644 --- a/superset/extensions/utils.py +++ b/superset/extensions/utils.py @@ -35,7 +35,12 @@ logger = logging.getLogger(__name__) -FRONTEND_REGEX = re.compile(r"^frontend/dist/([^/]+)$") +# Accept nested paths inside frontend/dist so extensions can serve +# worker / WASM / chunk subfolders. Reject any entry whose path contains "..", +# conservatively excluding parent traversal segments so a crafted entry name +# cannot escape the bundle directory (defense in depth; check_is_safe_zip runs +# first). +FRONTEND_REGEX = re.compile(r"^frontend/dist/(?!.*\.\.)(.+)$") # Reject any entry whose path contains "..", conservatively excluding parent # traversal segments along with the (in practice nonexistent) case of a module # path embedding consecutive dots, so a crafted entry name cannot produce a From b25ebcf2e38a6cfff1e8a431b0604c18334ce2c3 Mon Sep 17 00:00:00 2001 From: Claude Code Date: Tue, 19 May 2026 00:23:40 -0500 Subject: [PATCH 2/7] fix(extensions): address review on hot-reload watcher MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per @codeant-ai's and @bito's review on #40084: 1. First-edit dropped on startup. Pre-populate baseline hashes from existing files in watched `dist` dirs via `prime_baseline()`, called once at watcher startup. Real edits now diff against the on-disk baseline instead of being silently swallowed as "first observation". 2. Move events used src_path. Atomic-build workflows (webpack tmp + rename into `dist`) mean `src_path` may point outside the watched tree. Use `dest_path` for `FileMovedEvent`, falling back to `src_path`. 3. Moves trigger regardless of content match. A move into/out of `dist` is itself the signal — don't gate it on hashing the (potentially missing) source. 4. Leading-edge debounce replaced with trailing debounce via `threading.Timer`. Each event resets the timer so the reload fires once after the build settles, instead of triggering immediately and dropping the writes that finish the build. Co-Authored-By: Claude Sonnet 4.6 --- .../extensions/local_extensions_watcher.py | 120 +++++++++++++----- 1 file changed, 88 insertions(+), 32 deletions(-) diff --git a/superset/extensions/local_extensions_watcher.py b/superset/extensions/local_extensions_watcher.py index 4a6e3ad8f66c..2177d42b89ee 100644 --- a/superset/extensions/local_extensions_watcher.py +++ b/superset/extensions/local_extensions_watcher.py @@ -62,11 +62,17 @@ class LocalExtensionFileHandler(FileSystemEventHandler): def __init__(self) -> None: super().__init__() - # sha256 of last-seen content, keyed by absolute path + # sha256 of last-seen content, keyed by absolute path. Populated + # from existing files in watched `dist` dirs at startup (see + # `prime_baseline`) so that startup-noise inotify events from + # Docker VirtioFS reads don't get treated as the first real edit. self._file_hashes: dict[str, str] = {} - # Deduplicate: only trigger once per second across all files - self._last_trigger: float = 0.0 self._lock = threading.Lock() + # Trailing debounce: schedule a single reload after a quiet + # window so simultaneous webpack writes coalesce into one + # restart that fires *after* the build settles. + self._debounce_seconds = 1.0 + self._pending_timer: threading.Timer | None = None # ── helpers ────────────────────────────────────────────────────── @@ -78,24 +84,66 @@ def _sha256(path: str) -> str | None: except OSError: return None + def prime_baseline(self, watch_dirs: set[str]) -> None: + """Pre-populate content hashes for existing files in watched + `dist` directories. Called once at watcher startup so a + developer's first real edit registers as a content change + rather than as the file's 'first observation'.""" + for root_dir in watch_dirs: + root = Path(root_dir) + for path in root.rglob("*"): + if not path.is_file(): + continue + if "dist" not in path.parts: + continue + digest = self._sha256(str(path)) + if digest is not None: + self._file_hashes[str(path)] = digest + def _content_changed(self, path: str) -> bool: - """Return True only when the file's content differs from last seen. + """Return True when the file's content differs from last seen. - The first time a path is observed its hash is stored as the baseline - and False is returned — that event is a 'first-seen', not a change. - Only a subsequent event where the digest differs from the baseline - is treated as a genuine content change. + With `prime_baseline` called at startup, the baseline reflects + what was on disk when the watcher started. A first observation + that differs (or doesn't exist in baseline) is treated as a + genuine change. """ digest = self._sha256(path) if digest is None: return False old_digest = self._file_hashes.get(path) self._file_hashes[path] = digest - if old_digest is None: - # First observation — record baseline, do not trigger restart. - return False + # New file (not in baseline) is a real change; otherwise compare. return old_digest != digest + def _trigger_reload(self, source_path: str) -> None: + """Touch the reload-trigger sentinel; Flask's --extra-files + watcher reloads on its mtime change.""" + logger.info("File change settled in LOCAL_EXTENSIONS: %s", source_path) + logger.info("Triggering restart by touching %s", RELOAD_TRIGGER) + try: + os.utime(RELOAD_TRIGGER, (time.time(), time.time())) + except OSError as e: + logger.warning( + "Failed to touch reload trigger %s: %s", RELOAD_TRIGGER, e + ) + + def _schedule_reload(self, source_path: str) -> None: + """Trailing-debounce: cancel any pending reload and schedule a + new one for `_debounce_seconds` from now. Each new event resets + the timer, so the reload fires only after a quiet window.""" + with self._lock: + if self._pending_timer is not None: + self._pending_timer.cancel() + timer = threading.Timer( + self._debounce_seconds, + self._trigger_reload, + args=(source_path,), + ) + timer.daemon = True + self._pending_timer = timer + timer.start() + # ── event handler ───────────────────────────────────────────────── def on_any_event(self, event: Any) -> None: @@ -109,33 +157,37 @@ def on_any_event(self, event: Any) -> None: ): return - # Only care about files inside a `dist` directory - src = getattr(event, "src_path", None) - if not isinstance(src, str) or "dist" not in Path(src).parts: + # For atomic-build move workflows (e.g., webpack writing to + # tmp + rename into dist) the meaningful path is dest_path. + # For Create/Modify events watchdog only sets src_path. + if isinstance(event, FileMovedEvent): + target = getattr(event, "dest_path", None) or getattr( + event, "src_path", None + ) + else: + target = getattr(event, "src_path", None) + + if not isinstance(target, str): return - # Verify the file content actually changed to ignore spurious - # inotify events generated by Docker bind-mount reads. - if not self._content_changed(src): + # Only care about paths inside a `dist` directory. + if "dist" not in Path(target).parts: return - # Debounce: one restart per second max, regardless of how many - # files webpack writes simultaneously. - now = time.monotonic() - with self._lock: - if now - self._last_trigger < 1.0: - return - self._last_trigger = now + # Moves into/out of `dist` are explicit signals — trigger + # regardless of content match (the source may already be gone + # or the destination may not have a meaningful hash yet). + if isinstance(event, FileMovedEvent): + self._schedule_reload(target) + return - logger.info( - "File change detected in LOCAL_EXTENSIONS: %s", event.src_path - ) + # For Create/Modify, verify the content actually changed to + # ignore spurious inotify events generated by Docker bind-mount + # reads. + if not self._content_changed(target): + return - # Touch the dedicated reload-trigger sentinel file. - # Flask watches this via --extra-files; it is never read by Python - # so Docker VirtioFS will not generate spurious inotify events on it. - logger.info("Triggering restart by touching %s", RELOAD_TRIGGER) - os.utime(RELOAD_TRIGGER, (time.time(), time.time())) + self._schedule_reload(target) return LocalExtensionFileHandler except ImportError: @@ -219,6 +271,10 @@ def setup_local_extensions_watcher(app: Flask) -> None: # noqa: C901 # Set up and start the file watcher event_handler = handler_class() + # Pre-populate baseline hashes from existing dist files so the + # developer's first real edit isn't silently dropped as a "first + # observation". + event_handler.prime_baseline(watch_dirs) observer = Observer() for watch_dir in watch_dirs: From 1e2fd1ac82e57f87897d8071204a1a119cc8bf15 Mon Sep 17 00:00:00 2001 From: Evan Date: Sat, 4 Jul 2026 13:24:36 -0700 Subject: [PATCH 3/7] =?UTF-8?q?fix(extensions):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20type=20hints=20+=20trigger=20reload=20on=20moves=20?= =?UTF-8?q?out=20of=20dist?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Annotate the debounce interval and the frontend/backend bundle regexes, and treat a rename that moves a built artifact out of dist as a reload signal (previously only the dest path was checked, so moves out of dist were dropped despite the comment claiming they are handled). Co-Authored-By: Claude Opus 4.8 --- .../extensions/local_extensions_watcher.py | 37 +++++++++++-------- superset/extensions/utils.py | 4 +- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/superset/extensions/local_extensions_watcher.py b/superset/extensions/local_extensions_watcher.py index 2177d42b89ee..0d67d5f85c4e 100644 --- a/superset/extensions/local_extensions_watcher.py +++ b/superset/extensions/local_extensions_watcher.py @@ -71,7 +71,7 @@ def __init__(self) -> None: # Trailing debounce: schedule a single reload after a quiet # window so simultaneous webpack writes coalesce into one # restart that fires *after* the build settles. - self._debounce_seconds = 1.0 + self._debounce_seconds: float = 1.0 self._pending_timer: threading.Timer | None = None # ── helpers ────────────────────────────────────────────────────── @@ -157,16 +157,30 @@ def on_any_event(self, event: Any) -> None: ): return - # For atomic-build move workflows (e.g., webpack writing to - # tmp + rename into dist) the meaningful path is dest_path. - # For Create/Modify events watchdog only sets src_path. + # Moves into/out of `dist` are explicit signals — trigger + # regardless of content match (the source may already be gone + # or the destination may not have a meaningful hash yet). + # Atomic-build workflows rename tmp -> dist (dest in dist), + # while removing an artifact renames dist -> elsewhere (src in + # dist); either side touching `dist` is a real signal. if isinstance(event, FileMovedEvent): - target = getattr(event, "dest_path", None) or getattr( - event, "src_path", None + dest = getattr(event, "dest_path", None) + src = getattr(event, "src_path", None) + dist_side = next( + ( + p + for p in (dest, src) + if isinstance(p, str) and "dist" in Path(p).parts + ), + None, ) - else: - target = getattr(event, "src_path", None) + if dist_side is None: + return + self._schedule_reload(dist_side) + return + # For Create/Modify events watchdog only sets src_path. + target = getattr(event, "src_path", None) if not isinstance(target, str): return @@ -174,13 +188,6 @@ def on_any_event(self, event: Any) -> None: if "dist" not in Path(target).parts: return - # Moves into/out of `dist` are explicit signals — trigger - # regardless of content match (the source may already be gone - # or the destination may not have a meaningful hash yet). - if isinstance(event, FileMovedEvent): - self._schedule_reload(target) - return - # For Create/Modify, verify the content actually changed to # ignore spurious inotify events generated by Docker bind-mount # reads. diff --git a/superset/extensions/utils.py b/superset/extensions/utils.py index cb7a52abfbdb..82f7b475d7fa 100644 --- a/superset/extensions/utils.py +++ b/superset/extensions/utils.py @@ -40,12 +40,12 @@ # conservatively excluding parent traversal segments so a crafted entry name # cannot escape the bundle directory (defense in depth; check_is_safe_zip runs # first). -FRONTEND_REGEX = re.compile(r"^frontend/dist/(?!.*\.\.)(.+)$") +FRONTEND_REGEX: re.Pattern[str] = re.compile(r"^frontend/dist/(?!.*\.\.)(.+)$") # Reject any entry whose path contains "..", conservatively excluding parent # traversal segments along with the (in practice nonexistent) case of a module # path embedding consecutive dots, so a crafted entry name cannot produce a # traversal-style module path (defense in depth; check_is_safe_zip runs first). -BACKEND_REGEX = re.compile(r"^backend/src/(?!.*\.\.)(.+)$") +BACKEND_REGEX: re.Pattern[str] = re.compile(r"^backend/src/(?!.*\.\.)(.+)$") class InMemoryLoader(importlib.abc.Loader): From 591d6398000b16ceba8e09fd5f5597d2c2bd4d32 Mon Sep 17 00:00:00 2001 From: Evan Date: Sun, 5 Jul 2026 15:40:37 -0700 Subject: [PATCH 4/7] fix(extensions): annotate reload sentinel + guard debounce timer race Co-Authored-By: Claude Opus 4.8 --- superset/extensions/local_extensions_watcher.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/superset/extensions/local_extensions_watcher.py b/superset/extensions/local_extensions_watcher.py index 0d67d5f85c4e..4cd6c03bbac9 100644 --- a/superset/extensions/local_extensions_watcher.py +++ b/superset/extensions/local_extensions_watcher.py @@ -32,7 +32,7 @@ # Sentinel file Flask watches via --extra-files. Touching it on a real change # triggers a server reload without depending on cwd or the location of any # Python source file. -RELOAD_TRIGGER = Path(__file__).resolve().parent / ".reload_trigger" +RELOAD_TRIGGER: Path = Path(__file__).resolve().parent / ".reload_trigger" # Guard to prevent multiple initializations _watcher_initialized = False @@ -73,6 +73,10 @@ def __init__(self) -> None: # restart that fires *after* the build settles. self._debounce_seconds: float = 1.0 self._pending_timer: threading.Timer | None = None + # Monotonically increasing token identifying the most recently + # scheduled timer. Guards the timer-already-fired race where + # `Timer.cancel()` can't stop a callback that has begun running. + self._reload_generation: int = 0 # ── helpers ────────────────────────────────────────────────────── @@ -116,9 +120,15 @@ def _content_changed(self, path: str) -> bool: # New file (not in baseline) is a real change; otherwise compare. return old_digest != digest - def _trigger_reload(self, source_path: str) -> None: + def _trigger_reload(self, source_path: str, generation: int) -> None: """Touch the reload-trigger sentinel; Flask's --extra-files watcher reloads on its mtime change.""" + # A newer event may have superseded this timer after it began + # running (`cancel()` can't stop an in-flight callback), so only + # the most recently scheduled generation is allowed to fire. + with self._lock: + if generation != self._reload_generation: + return logger.info("File change settled in LOCAL_EXTENSIONS: %s", source_path) logger.info("Triggering restart by touching %s", RELOAD_TRIGGER) try: @@ -135,10 +145,11 @@ def _schedule_reload(self, source_path: str) -> None: with self._lock: if self._pending_timer is not None: self._pending_timer.cancel() + self._reload_generation += 1 timer = threading.Timer( self._debounce_seconds, self._trigger_reload, - args=(source_path,), + args=(source_path, self._reload_generation), ) timer.daemon = True self._pending_timer = timer From 1eba1443e13a14714af9b699229b9a94581ea365 Mon Sep 17 00:00:00 2001 From: Evan Date: Mon, 6 Jul 2026 10:39:25 -0700 Subject: [PATCH 5/7] fix(extensions): keep cache middleware in sync with nested asset paths + evict stale watcher hashes - Widen _ASSET_PATH_RE so nested extension asset paths (now allowed by the route) still get Vary: Cookie stripped - Evict _file_hashes entries on delete/move-out so the watcher's hash index doesn't grow unbounded as hashed chunk filenames churn across rebuilds - Extract _handle_moved to keep on_any_event under the complexity limit - Add unit tests for both behaviors Co-Authored-By: Claude Opus 4.8 --- superset/extensions/cache_middleware.py | 6 +- .../extensions/local_extensions_watcher.py | 56 ++++++++++++------- .../extensions/test_cache_middleware.py | 15 +++++ .../test_local_extensions_watcher.py | 55 ++++++++++++++++++ 4 files changed, 111 insertions(+), 21 deletions(-) create mode 100644 tests/unit_tests/extensions/test_local_extensions_watcher.py diff --git a/superset/extensions/cache_middleware.py b/superset/extensions/cache_middleware.py index e8a134052a00..f484db69f683 100644 --- a/superset/extensions/cache_middleware.py +++ b/superset/extensions/cache_middleware.py @@ -23,9 +23,11 @@ if TYPE_CHECKING: from _typeshed.wsgi import StartResponse, WSGIApplication, WSGIEnvironment -# Matches only the static asset endpoint: /api/v1/extensions/// +# Matches only the static asset endpoint: +# /api/v1/extensions///, where the file portion may +# contain nested segments (worker / WASM / chunk subfolders). # Does not match the list (/), get (//), or info (/_info) endpoints. -_ASSET_PATH_RE = re.compile(r"^/api/v1/extensions/[^/]+/[^/]+/[^/]+$") +_ASSET_PATH_RE = re.compile(r"^/api/v1/extensions/[^/]+/[^/]+/.+$") class ExtensionCacheMiddleware: diff --git a/superset/extensions/local_extensions_watcher.py b/superset/extensions/local_extensions_watcher.py index 4cd6c03bbac9..4f29ee1ba39d 100644 --- a/superset/extensions/local_extensions_watcher.py +++ b/superset/extensions/local_extensions_watcher.py @@ -46,6 +46,7 @@ def _get_file_handler_class() -> Any: # noqa: C901 from watchdog.events import ( FileCreatedEvent, + FileDeletedEvent, FileModifiedEvent, FileMovedEvent, FileSystemEventHandler, @@ -155,6 +156,30 @@ def _schedule_reload(self, source_path: str) -> None: self._pending_timer = timer timer.start() + def _handle_moved(self, event: Any) -> None: + """Moves into/out of `dist` are explicit signals — trigger + regardless of content match (the source may already be gone + or the destination may not have a meaningful hash yet). + Atomic-build workflows rename tmp -> dist (dest in dist), + while removing an artifact renames dist -> elsewhere (src in + dist); either side touching `dist` is a real signal.""" + dest = getattr(event, "dest_path", None) + src = getattr(event, "src_path", None) + # The file no longer lives at the source path; evict its + # hash entry so the index only tracks paths that exist. + if isinstance(src, str): + self._file_hashes.pop(src, None) + dist_side = next( + ( + p + for p in (dest, src) + if isinstance(p, str) and "dist" in Path(p).parts + ), + None, + ) + if dist_side is not None: + self._schedule_reload(dist_side) + # ── event handler ───────────────────────────────────────────────── def on_any_event(self, event: Any) -> None: @@ -162,32 +187,25 @@ def on_any_event(self, event: Any) -> None: if event.is_directory: return + # Deletions don't trigger a reload (webpack clean steps delete + # old chunks right before writing new ones, which trigger via + # the subsequent create/modify), but the stale hash entry must + # be evicted so `_file_hashes` doesn't grow without bound as + # hashed chunk filenames churn across rebuilds. + if isinstance(event, FileDeletedEvent): + src = getattr(event, "src_path", None) + if isinstance(src, str): + self._file_hashes.pop(src, None) + return + # Only react to true write events; skip access / close / open etc. if not isinstance( event, (FileCreatedEvent, FileModifiedEvent, FileMovedEvent) ): return - # Moves into/out of `dist` are explicit signals — trigger - # regardless of content match (the source may already be gone - # or the destination may not have a meaningful hash yet). - # Atomic-build workflows rename tmp -> dist (dest in dist), - # while removing an artifact renames dist -> elsewhere (src in - # dist); either side touching `dist` is a real signal. if isinstance(event, FileMovedEvent): - dest = getattr(event, "dest_path", None) - src = getattr(event, "src_path", None) - dist_side = next( - ( - p - for p in (dest, src) - if isinstance(p, str) and "dist" in Path(p).parts - ), - None, - ) - if dist_side is None: - return - self._schedule_reload(dist_side) + self._handle_moved(event) return # For Create/Modify events watchdog only sets src_path. diff --git a/tests/unit_tests/extensions/test_cache_middleware.py b/tests/unit_tests/extensions/test_cache_middleware.py index 22f8b5068200..c69783e3342a 100644 --- a/tests/unit_tests/extensions/test_cache_middleware.py +++ b/tests/unit_tests/extensions/test_cache_middleware.py @@ -64,6 +64,21 @@ def test_asset_path_is_intercepted() -> None: assert "Cookie" not in vary +def test_nested_asset_path_is_intercepted() -> None: + headers = call_middleware( + "/api/v1/extensions/acme/my-ext/workers/nested/chunk.wasm", + [("Vary", "Accept-Encoding, Cookie")], + ) + vary = dict(headers).get("Vary", "") + assert "Cookie" not in vary + + +def test_get_endpoint_with_trailing_slash_is_not_intercepted() -> None: + upstream = [("Vary", "Accept-Encoding, Cookie")] + headers = call_middleware("/api/v1/extensions/acme/my-ext/", upstream) + assert headers == upstream + + def test_list_endpoint_is_not_intercepted() -> None: upstream = [("Vary", "Accept-Encoding, Cookie")] headers = call_middleware("/api/v1/extensions/", upstream) diff --git a/tests/unit_tests/extensions/test_local_extensions_watcher.py b/tests/unit_tests/extensions/test_local_extensions_watcher.py new file mode 100644 index 000000000000..04c7cec3c26e --- /dev/null +++ b/tests/unit_tests/extensions/test_local_extensions_watcher.py @@ -0,0 +1,55 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from typing import Any + +from watchdog.events import FileDeletedEvent, FileMovedEvent + +from superset.extensions.local_extensions_watcher import _get_file_handler_class + + +def make_handler() -> Any: + handler_class = _get_file_handler_class() + handler = handler_class() + # Avoid spinning up real debounce timers in unit tests. + handler._schedule_reload = lambda _path: None + return handler + + +def test_delete_evicts_hash_entry() -> None: + handler = make_handler() + handler._file_hashes["/ext/dist/old-chunk.abc123.js"] = "digest" + + handler.on_any_event(FileDeletedEvent("/ext/dist/old-chunk.abc123.js")) + + assert "/ext/dist/old-chunk.abc123.js" not in handler._file_hashes + + +def test_delete_of_untracked_path_is_a_noop() -> None: + handler = make_handler() + + handler.on_any_event(FileDeletedEvent("/ext/dist/never-seen.js")) + + assert handler._file_hashes == {} + + +def test_move_out_of_dist_evicts_source_hash_entry() -> None: + handler = make_handler() + handler._file_hashes["/ext/dist/chunk.js"] = "digest" + + handler.on_any_event(FileMovedEvent("/ext/dist/chunk.js", "/ext/tmp/chunk.js")) + + assert "/ext/dist/chunk.js" not in handler._file_hashes From cc1b46943a595bf87c11f2e9c2ec93953dac68cc Mon Sep 17 00:00:00 2001 From: Evan Date: Mon, 6 Jul 2026 18:28:48 -0700 Subject: [PATCH 6/7] fix(extensions): annotate lock/regex types + tighten reload race Co-Authored-By: Claude Opus 4.8 --- superset/extensions/cache_middleware.py | 2 +- .../extensions/local_extensions_watcher.py | 24 ++++++++++++------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/superset/extensions/cache_middleware.py b/superset/extensions/cache_middleware.py index f484db69f683..c688bbe8d9fb 100644 --- a/superset/extensions/cache_middleware.py +++ b/superset/extensions/cache_middleware.py @@ -27,7 +27,7 @@ # /api/v1/extensions///, where the file portion may # contain nested segments (worker / WASM / chunk subfolders). # Does not match the list (/), get (//), or info (/_info) endpoints. -_ASSET_PATH_RE = re.compile(r"^/api/v1/extensions/[^/]+/[^/]+/.+$") +_ASSET_PATH_RE: re.Pattern[str] = re.compile(r"^/api/v1/extensions/[^/]+/[^/]+/.+$") class ExtensionCacheMiddleware: diff --git a/superset/extensions/local_extensions_watcher.py b/superset/extensions/local_extensions_watcher.py index 4f29ee1ba39d..a2908f941ac8 100644 --- a/superset/extensions/local_extensions_watcher.py +++ b/superset/extensions/local_extensions_watcher.py @@ -68,7 +68,7 @@ def __init__(self) -> None: # `prime_baseline`) so that startup-noise inotify events from # Docker VirtioFS reads don't get treated as the first real edit. self._file_hashes: dict[str, str] = {} - self._lock = threading.Lock() + self._lock: threading.Lock = threading.Lock() # Trailing debounce: schedule a single reload after a quiet # window so simultaneous webpack writes coalesce into one # restart that fires *after* the build settles. @@ -126,18 +126,24 @@ def _trigger_reload(self, source_path: str, generation: int) -> None: watcher reloads on its mtime change.""" # A newer event may have superseded this timer after it began # running (`cancel()` can't stop an in-flight callback), so only - # the most recently scheduled generation is allowed to fire. + # the most recently scheduled generation is allowed to fire. The + # check and the sentinel touch happen inside the same critical + # section so a `_schedule_reload` call racing in from another + # thread can't bump the generation between the check and the + # write and let this stale callback still fire. with self._lock: if generation != self._reload_generation: return - logger.info("File change settled in LOCAL_EXTENSIONS: %s", source_path) - logger.info("Triggering restart by touching %s", RELOAD_TRIGGER) - try: - os.utime(RELOAD_TRIGGER, (time.time(), time.time())) - except OSError as e: - logger.warning( - "Failed to touch reload trigger %s: %s", RELOAD_TRIGGER, e + logger.info( + "File change settled in LOCAL_EXTENSIONS: %s", source_path ) + logger.info("Triggering restart by touching %s", RELOAD_TRIGGER) + try: + os.utime(RELOAD_TRIGGER, (time.time(), time.time())) + except OSError as e: + logger.warning( + "Failed to touch reload trigger %s: %s", RELOAD_TRIGGER, e + ) def _schedule_reload(self, source_path: str) -> None: """Trailing-debounce: cancel any pending reload and schedule a From dae914ed6fa36f1bafad6cc73dcf617aeaae4bbb Mon Sep 17 00:00:00 2001 From: Evan Date: Mon, 6 Jul 2026 21:06:51 -0700 Subject: [PATCH 7/7] test(extensions): annotate local test vars flagged by codeant review Co-Authored-By: Claude Opus 4.8 --- tests/unit_tests/extensions/test_cache_middleware.py | 6 +++--- .../unit_tests/extensions/test_local_extensions_watcher.py | 6 +++++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/unit_tests/extensions/test_cache_middleware.py b/tests/unit_tests/extensions/test_cache_middleware.py index c69783e3342a..e9398032d68c 100644 --- a/tests/unit_tests/extensions/test_cache_middleware.py +++ b/tests/unit_tests/extensions/test_cache_middleware.py @@ -65,16 +65,16 @@ def test_asset_path_is_intercepted() -> None: def test_nested_asset_path_is_intercepted() -> None: - headers = call_middleware( + headers: ResponseHeaders = call_middleware( "/api/v1/extensions/acme/my-ext/workers/nested/chunk.wasm", [("Vary", "Accept-Encoding, Cookie")], ) - vary = dict(headers).get("Vary", "") + vary: str = dict(headers).get("Vary", "") assert "Cookie" not in vary def test_get_endpoint_with_trailing_slash_is_not_intercepted() -> None: - upstream = [("Vary", "Accept-Encoding, Cookie")] + upstream: ResponseHeaders = [("Vary", "Accept-Encoding, Cookie")] headers = call_middleware("/api/v1/extensions/acme/my-ext/", upstream) assert headers == upstream diff --git a/tests/unit_tests/extensions/test_local_extensions_watcher.py b/tests/unit_tests/extensions/test_local_extensions_watcher.py index 04c7cec3c26e..85f2a2bdaacd 100644 --- a/tests/unit_tests/extensions/test_local_extensions_watcher.py +++ b/tests/unit_tests/extensions/test_local_extensions_watcher.py @@ -21,11 +21,15 @@ from superset.extensions.local_extensions_watcher import _get_file_handler_class +def _noop_schedule_reload(_path: str) -> None: + """Stand in for a real debounce timer in unit tests.""" + + def make_handler() -> Any: handler_class = _get_file_handler_class() handler = handler_class() # Avoid spinning up real debounce timers in unit tests. - handler._schedule_reload = lambda _path: None + handler._schedule_reload = _noop_schedule_reload return handler