From 639b7654d6b0fad9e8ad7ba325b6984a45fc7074 Mon Sep 17 00:00:00 2001 From: Mirko Dietrich Date: Tue, 4 Jun 2024 16:07:14 +0200 Subject: [PATCH] feat: watch mode for `run` command Closes #101 --- poetry.lock | 6 +- pyproject.toml | 2 +- questionpy_sdk/commands/_helper.py | 32 ++++----- questionpy_sdk/commands/run.py | 28 +++++++- questionpy_sdk/package/builder.py | 2 +- questionpy_sdk/package/watcher.py | 111 +++++++++++++++++++++++++++++ questionpy_sdk/webserver/app.py | 46 ++++++++++-- tests/package/test_watcher.py | 75 +++++++++++++++++++ 8 files changed, 269 insertions(+), 33 deletions(-) create mode 100644 questionpy_sdk/package/watcher.py create mode 100644 tests/package/test_watcher.py diff --git a/poetry.lock b/poetry.lock index 6bed258f..c41fbf9d 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1220,8 +1220,8 @@ watchdog = "^4.0.0" [package.source] type = "git" url = "https://github.com/questionpy-org/questionpy-server.git" -reference = "763c15bcf3906ebb80d8d0bd9c15e842de4f8b0c" -resolved_reference = "763c15bcf3906ebb80d8d0bd9c15e842de4f8b0c" +reference = "13c28ea4885b896f45f5183b580bbc3f182a11d8" +resolved_reference = "13c28ea4885b896f45f5183b580bbc3f182a11d8" [[package]] name = "ruff" @@ -1551,4 +1551,4 @@ multidict = ">=4.0" [metadata] lock-version = "2.0" python-versions = "^3.11" -content-hash = "f1da0050fb6b89dc4e61b25719cf0363b1a7e8052c8c8322732bdb2f5f8046d6" +content-hash = "20ead7e8b0fbdebe8380e654c8a8031fb160027faeb043b66c84fd98208fa751" diff --git a/pyproject.toml b/pyproject.toml index 8fb50139..3a21a22e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,7 +28,7 @@ python = "^3.11" aiohttp = "^3.9.3" pydantic = "^2.6.4" PyYAML = "^6.0.1" -questionpy-server = { git = "https://github.com/questionpy-org/questionpy-server.git", rev = "763c15bcf3906ebb80d8d0bd9c15e842de4f8b0c" } +questionpy-server = { git = "https://github.com/questionpy-org/questionpy-server.git", rev = "13c28ea4885b896f45f5183b580bbc3f182a11d8" } jinja2 = "^3.1.3" aiohttp-jinja2 = "^1.6" lxml = "~5.1.0" diff --git a/questionpy_sdk/commands/_helper.py b/questionpy_sdk/commands/_helper.py index e5738a3c..e00c0b09 100644 --- a/questionpy_sdk/commands/_helper.py +++ b/questionpy_sdk/commands/_helper.py @@ -21,7 +21,8 @@ ) -def build_dir_package(source_path: Path) -> None: +def _get_dir_package_location(pkg_string: str, source_path: Path) -> DirPackageLocation: + # Always rebuild package. try: package_source = PackageSource(source_path) except PackageSourceValidationError as exc: @@ -34,28 +35,23 @@ def build_dir_package(source_path: Path) -> None: msg = f"Failed to build package: {exc}" raise click.ClickException(msg) from exc + click.echo(f"Successfully built package '{pkg_string}'.") + + try: + manifest_path = source_path / DIST_DIR / MANIFEST_FILENAME + with manifest_path.open() as manifest_fp: + manifest = Manifest.model_validate_json(manifest_fp.read()) + return DirPackageLocation(source_path, manifest) + except (OSError, ValidationError, ValueError) as exc: + msg = f"Failed to read package manifest:\n{exc}" + raise click.ClickException(msg) from exc + def infer_package_kind(pkg_string: str) -> PackageLocation: pkg_path = Path(pkg_string) if pkg_path.is_dir(): - try: - manifest_path = pkg_path / DIST_DIR / MANIFEST_FILENAME - try: - manifest_fp = manifest_path.open() - except FileNotFoundError: - # build package if needed - build_dir_package(pkg_path) - click.echo(f"Successfully built package '{pkg_string}'.") - manifest_fp = manifest_path.open() - manifest = Manifest.model_validate_json(manifest_fp.read()) - return DirPackageLocation(pkg_path, manifest) - except (OSError, ValidationError, ValueError) as exc: - msg = f"Failed to read package manifest:\n{exc}" - raise click.ClickException(msg) from exc - finally: - if manifest_fp: - manifest_fp.close() + return _get_dir_package_location(pkg_string, pkg_path) if zipfile.is_zipfile(pkg_path): return ZipPackageLocation(pkg_path) diff --git a/questionpy_sdk/commands/run.py b/questionpy_sdk/commands/run.py index c393a7be..b3b3b115 100644 --- a/questionpy_sdk/commands/run.py +++ b/questionpy_sdk/commands/run.py @@ -5,11 +5,33 @@ import click from questionpy_sdk.commands._helper import infer_package_kind +from questionpy_sdk.package.watcher import Watcher from questionpy_sdk.webserver.app import WebServer +from questionpy_server.worker.runtime.package_location import DirPackageLocation @click.command() @click.argument("package") -def run(package: str) -> None: - web_server = WebServer(infer_package_kind(package)) - web_server.start_server() +@click.option("--watch", "-w", "watch", is_flag=True, help="Watch source directory and rebuild on changes.") +def run(package: str, *, watch: bool) -> None: + """Run a package. + + \b + PACKAGE can be: + - a .qpy file, + - a dist directory, or + - a source directory (built on-the-fly). + """ # noqa: D301 + pkg_location = infer_package_kind(package) + web_server = WebServer(pkg_location) + + if watch: + if not isinstance(pkg_location, DirPackageLocation): + msg = "The --watch option only works with source directories." + raise click.BadParameter(msg) + + with Watcher(pkg_location.path, web_server): + web_server.start_server() + + else: + web_server.start_server() diff --git a/questionpy_sdk/package/builder.py b/questionpy_sdk/package/builder.py index 824e865e..179ccb2d 100644 --- a/questionpy_sdk/package/builder.py +++ b/questionpy_sdk/package/builder.py @@ -23,7 +23,7 @@ from questionpy_sdk.package.errors import PackageBuildError from questionpy_sdk.package.source import PackageSource -log = logging.getLogger(__name__) +log = logging.getLogger("questionpy-sdk:builder") class PackageBuilderBase(AbstractContextManager): diff --git a/questionpy_sdk/package/watcher.py b/questionpy_sdk/package/watcher.py new file mode 100644 index 00000000..80aaaa74 --- /dev/null +++ b/questionpy_sdk/package/watcher.py @@ -0,0 +1,111 @@ +import asyncio +import logging +from contextlib import AbstractContextManager +from pathlib import Path +from types import TracebackType +from typing import Self + +from watchdog.events import ( + FileClosedEvent, + FileOpenedEvent, + FileSystemEvent, + FileSystemEventHandler, + FileSystemMovedEvent, +) +from watchdog.observers import Observer +from watchdog.utils.event_debouncer import EventDebouncer + +from questionpy_common.constants import DIST_DIR +from questionpy_sdk.package.builder import DirPackageBuilder +from questionpy_sdk.package.errors import PackageBuildError, PackageSourceValidationError +from questionpy_sdk.package.source import PackageSource +from questionpy_sdk.webserver.app import WebServer + +log = logging.getLogger("questionpy-sdk:watcher") + + +class _EventHandler(FileSystemEventHandler): + DEBOUNCE_INTERVAL = 0.5 + + def __init__(self, path: Path, web_server: WebServer) -> None: + self._path = path + self._web_server = web_server + self._event_debouncer = EventDebouncer(self.DEBOUNCE_INTERVAL, self._on_file_changes) + + def start(self) -> None: + self._event_debouncer.start() + + def stop(self) -> None: + self._event_debouncer.stop() + + def dispatch(self, event: FileSystemEvent) -> None: + if self._ignore_event(event): + return + + self._event_debouncer.handle_event(event) + + def _on_file_changes(self, events: list[FileSystemEvent]) -> None: + log.info("File changes detected. Rebuilding package...") + + try: + # build package + package_source = PackageSource(self._path) + with DirPackageBuilder(package_source) as builder: + builder.write_package() + + # reload web server's package location + fut = asyncio.run_coroutine_threadsafe(self._web_server.reload_package(), self._web_server.loop) + try: + fut.result(5) + except Exception: + log.exception("Failed to reload package. The exception was:") + + except (PackageBuildError, PackageSourceValidationError): + log.exception("Failed to build package. The exception was:") + + def _ignore_event(self, event: FileSystemEvent) -> bool: + """Ignores events that should not trigger a rebuild. + + Args: + event: The event to check. + + Returns: + `True` if event should be ignored, otherwise `False`. + """ + if isinstance(event, FileOpenedEvent | FileClosedEvent): + return True + + # ignore events happening under `dist` dir + relevant_path = event.dest_path if isinstance(event, FileSystemMovedEvent) else event.src_path + try: + return Path(relevant_path).relative_to(self._path).parts[0] == DIST_DIR + except IndexError: + return False + + +class Watcher(AbstractContextManager): + def __init__(self, path: Path, web_server: WebServer) -> None: + self._path = path + self._web_server = web_server + self._event_handler = _EventHandler(path.absolute(), self._web_server) + self._observer = Observer() + + def __enter__(self) -> Self: + self.start() + return self + + def __exit__( + self, exc_type: type[BaseException] | None, exc_value: BaseException | None, traceback: TracebackType | None + ) -> None: + self.stop() + + def start(self) -> None: + self._event_handler.start() + self._observer.schedule(self._event_handler, self._path.absolute(), recursive=True) + self._observer.start() + log.info("Watching '%s' for changes...", self._path) + + def stop(self) -> None: + log.debug("Shutting down...") + self._observer.stop() + self._event_handler.stop() diff --git a/questionpy_sdk/webserver/app.py b/questionpy_sdk/webserver/app.py index ea27246b..7b48f7a7 100644 --- a/questionpy_sdk/webserver/app.py +++ b/questionpy_sdk/webserver/app.py @@ -1,6 +1,8 @@ # This file is part of the QuestionPy SDK. (https://questionpy.org) # The QuestionPy SDK is free software released under terms of the MIT license. See LICENSE.md. # (c) Technische Universität Berlin, innoCampus +import asyncio +import logging import traceback from functools import cached_property from pathlib import Path @@ -12,15 +14,17 @@ from jinja2 import PackageLoader from questionpy_common.api.qtype import InvalidQuestionStateError -from questionpy_common.constants import MiB +from questionpy_common.constants import DIST_DIR, MANIFEST_FILENAME, MiB from questionpy_common.manifest import Manifest from questionpy_server import WorkerPool -from questionpy_server.worker.runtime.package_location import PackageLocation +from questionpy_server.worker.runtime.package_location import DirPackageLocation, PackageLocation from questionpy_server.worker.worker.thread import ThreadWorker if TYPE_CHECKING: from questionpy_server.worker.worker import Worker +log = logging.getLogger("questionpy-sdk:web-server") + async def _extract_manifest(app: web.Application) -> None: webserver = app[SDK_WEBSERVER_APP_KEY] @@ -47,6 +51,7 @@ def __init__( self, package_location: PackageLocation, state_storage_path: Path = Path(__file__).parent / "question_state_storage", + loop: asyncio.AbstractEventLoop | None = None, ) -> None: # We import here, so we don't have to work around circular imports. from questionpy_sdk.webserver.routes.attempt import routes as attempt_routes # noqa: PLC0415 @@ -56,6 +61,10 @@ def __init__( self.package_location = package_location self._state_storage_path = state_storage_path + if loop is None: + loop = asyncio.new_event_loop() + self._loop = loop + self.web_app = web.Application() self.web_app[SDK_WEBSERVER_APP_KEY] = self @@ -76,10 +85,10 @@ def save_question_state(self, question_state: str) -> None: self._state_file_path.write_text(question_state) def load_question_state(self) -> str | None: - path = self._state_file_path - if path.exists(): - return path.read_text() - return None + try: + return self._state_file_path.read_text() + except FileNotFoundError: + return None def delete_question_state(self) -> None: self._state_file_path.unlink(missing_ok=True) @@ -89,8 +98,31 @@ def _state_file_path(self) -> Path: manifest = self.web_app[MANIFEST_APP_KEY] return self._state_storage_path / f"{manifest.namespace}-{manifest.short_name}-{manifest.version}.txt" + @property + def loop(self) -> asyncio.AbstractEventLoop: + return self._loop + def start_server(self) -> None: - web.run_app(self.web_app) + web.run_app(self.web_app, loop=self._loop) + + async def reload_package(self) -> None: + log.info("Reloading package location...") + + if not isinstance(self.package_location, DirPackageLocation): + msg = "reload_pkg_location only works with DirPackageLocation" + raise TypeError(msg) + + pkg_path = self.package_location.path + manifest = Manifest.model_validate_json((pkg_path / DIST_DIR / MANIFEST_FILENAME).read_text()) + + worker: Worker + async with self.worker_pool.get_worker(self.package_location, 0, None) as worker: + # reload package in worker + await worker.load_package(reload=True) + + self.web_app[MANIFEST_APP_KEY] = manifest + self.package_location = DirPackageLocation(pkg_path, manifest) + self.delete_question_state() SDK_WEBSERVER_APP_KEY = web.AppKey("sdk_webserver_app", WebServer) diff --git a/tests/package/test_watcher.py b/tests/package/test_watcher.py new file mode 100644 index 00000000..df39c531 --- /dev/null +++ b/tests/package/test_watcher.py @@ -0,0 +1,75 @@ +# This file is part of the QuestionPy SDK. (https://questionpy.org) +# The QuestionPy SDK is free software released under terms of the MIT license. See LICENSE.md. +# (c) Technische Universität Berlin, innoCampus + +from pathlib import Path +from typing import cast + +import pytest +from watchdog.events import ( + DirCreatedEvent, + DirDeletedEvent, + DirModifiedEvent, + DirMovedEvent, + FileClosedEvent, + FileCreatedEvent, + FileDeletedEvent, + FileModifiedEvent, + FileMovedEvent, + FileOpenedEvent, + FileSystemEvent, +) + +from questionpy_common.constants import DIST_DIR +from questionpy_sdk.package.watcher import _EventHandler +from questionpy_sdk.webserver.app import WebServer + +path = Path("/", "path", "to") + + +@pytest.fixture +def event_handler() -> _EventHandler: + class MockWebServer: + loop = "mock value" + + return _EventHandler(path, cast(WebServer, MockWebServer())) + + +@pytest.mark.parametrize( + "event", + [ + DirCreatedEvent(src_path=str(path / "foo")), + DirDeletedEvent(src_path=str(path / "foo")), + DirModifiedEvent(src_path=str(path / "foo")), + DirMovedEvent(src_path=str(path / DIST_DIR / "foo"), dest_path=str(path / "foo")), + FileCreatedEvent(src_path=str(path / "foo")), + FileCreatedEvent(src_path=str(path / "python" / "foo" / "bar" / "module.py")), + FileDeletedEvent(src_path=str(path / "foo")), + FileDeletedEvent(src_path=str(path / "python" / "foo" / "bar" / "module.py")), + FileModifiedEvent(src_path=str(path)), + FileModifiedEvent(src_path=str(path / "python" / "foo" / "bar" / "module.py")), + FileMovedEvent(src_path=str(path / DIST_DIR / "foo"), dest_path=str(path / "foo")), + ], +) +def test_should_not_ignore_events(event: FileSystemEvent, event_handler: _EventHandler) -> None: + assert not event_handler._ignore_event(event) + + +# test that the watcher is ignoring certain events, like moving a file into the `dist` folder +@pytest.mark.parametrize( + "event", + [ + DirCreatedEvent(src_path=str(path / DIST_DIR / "foo")), + DirDeletedEvent(src_path=str(path / DIST_DIR / "foo")), + DirModifiedEvent(src_path=str(path / DIST_DIR / "foo")), + FileClosedEvent(src_path=str(path / "foo")), + DirMovedEvent(src_path=str(path / "foo"), dest_path=str(path / DIST_DIR / "foo")), + FileCreatedEvent(src_path=str(path / DIST_DIR / "foo")), + FileDeletedEvent(src_path=str(path / DIST_DIR / "foo")), + FileModifiedEvent(src_path=str(path / DIST_DIR)), + FileMovedEvent(src_path=str(path / "foo"), dest_path=str(path / DIST_DIR / "foo")), + FileOpenedEvent(src_path=str(path / "foo")), + ], +) +def test_should_ignore_events(event: FileSystemEvent, event_handler: _EventHandler) -> None: + assert event_handler._ignore_event(event)