diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 18495c25..a8b040e0 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -2,6 +2,7 @@ repos: - repo: https://github.com/pre-commit/pre-commit-hooks rev: v4.4.0 hooks: + - id: trailing-whitespace - id: check-yaml - id: check-toml @@ -9,15 +10,28 @@ repos: rev: 5.12.0 hooks: - id: isort - additional_dependencies: - - toml + additional_dependencies: [toml] - repo: https://github.com/psf/black rev: 23.3.0 hooks: - id: black - - repo: https://github.com/PyCQA/flake8 + - repo: https://github.com/asottile/blacken-docs + rev: 1.13.0 + hooks: + - id: blacken-docs + additional_dependencies: [black==23.3.0] + args: [-l, '79', -t, py311] + + - repo: https://github.com/pycqa/flake8 rev: 6.0.0 hooks: - id: flake8 + + - repo: https://github.com/pycqa/pydocstyle + rev: 6.3.0 + hooks: + - id: pydocstyle + additional_dependencies: [toml] + files: ^src/ diff --git a/Makefile b/Makefile index cdfd6dee..959ddfee 100644 --- a/Makefile +++ b/Makefile @@ -25,4 +25,4 @@ update: update-deps init .PHONY: run run: - adev runserver --app-factory create_app src/mobu/app.py + tox run -e run diff --git a/changelog.d/20230510_082737_rra_DM_38425.md b/changelog.d/20230510_082737_rra_DM_38425.md new file mode 100644 index 00000000..df828e36 --- /dev/null +++ b/changelog.d/20230510_082737_rra_DM_38425.md @@ -0,0 +1,3 @@ +### New features + +- The prefix for mobu routes (`/mobu` by default) can now be configured with `SAFIR_PATH_PREFIX`. diff --git a/changelog.d/20230510_083114_rra_DM_38425.md b/changelog.d/20230510_083114_rra_DM_38425.md new file mode 100644 index 00000000..d7435cba --- /dev/null +++ b/changelog.d/20230510_083114_rra_DM_38425.md @@ -0,0 +1,3 @@ +### New features + +- Uncaught exceptions from mobu's route handlers are now also reported to Slack. diff --git a/pyproject.toml b/pyproject.toml index 05771c64..590f828a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -81,6 +81,19 @@ line_length = 79 known_first_party = ["mobu", "tests"] skip = ["docs/conf.py"] +[tool.pydocstyle] +# Reference: https://www.pydocstyle.org/en/stable/error_codes.html +convention = "numpy" +add-ignore = [ + "D105", # Missing docstring in magic method + "D104", # Missing docstring in public package + "D102", # Missing docstring in public method (docstring inheritance) + "D100", # Missing docstring in public module + # Nested classes named Config are used by Pydantic for configuration, and + # don't have a meaningful docstring. + "D106", +] + [tool.pytest.ini_options] asyncio_mode = "strict" filterwarnings = [ diff --git a/src/mobu/config.py b/src/mobu/config.py index caed33a6..5f48500a 100644 --- a/src/mobu/config.py +++ b/src/mobu/config.py @@ -104,6 +104,12 @@ class Configuration(BaseSettings): env="SAFIR_NAME", ) + path_prefix: str = Field( + "/mobu", + title="URL prefix for application API", + env="SAFIR_PATH_PREFIX", + ) + profile: Profile = Field( Profile.development, title="Application logging profile", diff --git a/src/mobu/exceptions.py b/src/mobu/exceptions.py index e4b1439e..8731238c 100644 --- a/src/mobu/exceptions.py +++ b/src/mobu/exceptions.py @@ -6,8 +6,11 @@ from datetime import datetime from typing import Optional, Self +from fastapi import status from httpx_ws import HTTPXWSException, WebSocketDisconnect from safir.datetime import format_datetime_for_logging +from safir.fastapi import ClientRequestError +from safir.models import ErrorLocation from safir.slack.blockkit import ( SlackBaseBlock, SlackBaseField, @@ -25,13 +28,13 @@ __all__ = [ "CachemachineError", "CodeExecutionError", - "FlockNotFoundException", + "FlockNotFoundError", "GafaelfawrWebError", "JupyterTimeoutError", "JupyterWebError", "MobuSlackException", "MobuSlackWebException", - "MonkeyNotFoundException", + "MonkeyNotFoundError", "TAPClientError", ] @@ -63,6 +66,30 @@ class GafaelfawrWebError(SlackWebException): """An API call to Gafaelfawr failed.""" +class FlockNotFoundError(ClientRequestError): + """The named flock was not found.""" + + error = "flock_not_found" + status_code = status.HTTP_404_NOT_FOUND + + def __init__(self, flock: str) -> None: + self.flock = flock + msg = f"Flock {flock} not found" + super().__init__(msg, ErrorLocation.path, ["flock"]) + + +class MonkeyNotFoundError(ClientRequestError): + """The named monkey was not found.""" + + error = "monkey_not_found" + status_code = status.HTTP_404_NOT_FOUND + + def __init__(self, monkey: str) -> None: + self.monkey = monkey + msg = f"Monkey {monkey} not found" + super().__init__(msg, ErrorLocation.path, ["monkey"]) + + class MobuSlackException(SlackException): """Represents an exception that can be reported to Slack. @@ -159,22 +186,6 @@ class MobuSlackWebException(SlackWebException, MobuSlackException): """ -class FlockNotFoundException(Exception): - """The named flock was not found.""" - - def __init__(self, flock: str) -> None: - self.flock = flock - super().__init__(f"Flock {flock} not found") - - -class MonkeyNotFoundException(Exception): - """The named monkey was not found.""" - - def __init__(self, monkey: str) -> None: - self.monkey = monkey - super().__init__(f"Monkey {monkey} not found") - - class NotebookRepositoryError(MobuSlackException): """The repository containing notebooks to run is not valid.""" diff --git a/src/mobu/handlers/external.py b/src/mobu/handlers/external.py index d9bcd5f1..abf67e59 100644 --- a/src/mobu/handlers/external.py +++ b/src/mobu/handlers/external.py @@ -8,6 +8,7 @@ from safir.datetime import current_datetime from safir.metadata import get_metadata from safir.models import ErrorModel +from safir.slack.webhook import SlackRouteErrorHandler from ..config import config from ..dependencies.context import RequestContext, context_dependency @@ -16,9 +17,11 @@ from ..models.monkey import MonkeyData from ..models.solitary import SolitaryConfig, SolitaryResult -external_router = APIRouter() +external_router = APIRouter(route_class=SlackRouteErrorHandler) """FastAPI router for all external handlers.""" +__all__ = ["external_router"] + class FormattedJSONResponse(JSONResponse): """The same as ``fastapi.JSONResponse`` except formatted for humans.""" diff --git a/src/mobu/handlers/internal.py b/src/mobu/handlers/internal.py index 90562c73..e9872c4a 100644 --- a/src/mobu/handlers/internal.py +++ b/src/mobu/handlers/internal.py @@ -9,14 +9,15 @@ from fastapi import APIRouter from safir.metadata import Metadata, get_metadata +from safir.slack.webhook import SlackRouteErrorHandler from ..config import config -__all__ = ["get_index", "internal_router"] - -internal_router = APIRouter() +internal_router = APIRouter(route_class=SlackRouteErrorHandler) """FastAPI router for all internal handlers.""" +__all__ = ["internal_router"] + @internal_router.get( "/", diff --git a/src/mobu/main.py b/src/mobu/main.py index 21a8316f..d76de3f9 100644 --- a/src/mobu/main.py +++ b/src/mobu/main.py @@ -10,25 +10,25 @@ import asyncio from importlib.metadata import metadata, version -from fastapi import FastAPI, Request, status -from fastapi.responses import JSONResponse +import structlog +from fastapi import FastAPI +from safir.fastapi import ClientRequestError, client_request_error_handler from safir.logging import Profile, configure_logging, configure_uvicorn_logging from safir.middleware.x_forwarded import XForwardedMiddleware -from safir.models import ErrorLocation +from safir.slack.webhook import SlackRouteErrorHandler from .asyncio import schedule_periodic from .config import config from .dependencies.context import context_dependency -from .exceptions import FlockNotFoundException, MonkeyNotFoundException from .handlers.external import external_router from .handlers.internal import internal_router from .status import post_status -__all__ = ["app", "config"] +__all__ = ["app"] configure_logging( - profile=config.profile, log_level=config.log_level, name="mobu" + name="mobu", profile=config.profile, log_level=config.log_level ) if config.profile == Profile.production: configure_uvicorn_logging(config.log_level) @@ -37,19 +37,28 @@ title="mobu", description=metadata("mobu")["Summary"], version=version("mobu"), - openapi_url=f"/{config.name}/openapi.json", - docs_url=f"/{config.name}/docs", - redoc_url=f"/{config.name}/redoc", + openapi_url=f"{config.path_prefix}/openapi.json", + docs_url=f"{config.path_prefix}/docs", + redoc_url=f"{config.path_prefix}/redoc", ) """The main FastAPI application for mobu.""" # Attach the routers. app.include_router(internal_router) -app.include_router(external_router, prefix=f"/{config.name}") +app.include_router(external_router, prefix=config.path_prefix) # Add middleware. app.add_middleware(XForwardedMiddleware) +# Enable Slack alerting for uncaught exceptions. +if config.alert_hook: + logger = structlog.get_logger("mobu") + SlackRouteErrorHandler.initialize(config.alert_hook, "mobu", logger) + logger.debug("Initialized Slack webhook") + +# Enable the generic exception handler for client errors. +app.exception_handler(ClientRequestError)(client_request_error_handler) + @app.on_event("startup") async def startup_event() -> None: @@ -70,39 +79,3 @@ async def shutdown_event() -> None: await app.state.periodic_status except asyncio.CancelledError: pass - - -@app.exception_handler(FlockNotFoundException) -async def flock_not_found_exception_handler( - request: Request, exc: FlockNotFoundException -) -> JSONResponse: - return JSONResponse( - status_code=status.HTTP_404_NOT_FOUND, - content={ - "detail": [ - { - "loc": [ErrorLocation.path, "flock"], - "msg": f"Flock for {exc.flock} not found", - "type": "flock_not_found", - } - ] - }, - ) - - -@app.exception_handler(MonkeyNotFoundException) -async def monkey_not_found_exception_handler( - request: Request, exc: MonkeyNotFoundException -) -> JSONResponse: - return JSONResponse( - status_code=status.HTTP_404_NOT_FOUND, - content={ - "detail": [ - { - "loc": [ErrorLocation.path, "monkey"], - "msg": f"Monkey for {exc.monkey} not found", - "type": "monkey_not_found", - } - ] - }, - ) diff --git a/src/mobu/services/business/base.py b/src/mobu/services/business/base.py index 168883df..09419ef6 100644 --- a/src/mobu/services/business/base.py +++ b/src/mobu/services/business/base.py @@ -105,7 +105,7 @@ async def startup(self) -> None: @abstractmethod async def execute(self) -> None: - """The business done in each loop.""" + """Execute the core of each business loop.""" async def close(self) -> None: """Clean up any allocated resources. @@ -116,7 +116,7 @@ async def close(self) -> None: pass async def shutdown(self) -> None: - """Any cleanup to do before exiting after stopping.""" + """Perform any cleanup required after stopping.""" pass # Public Business API called by the Monkey class. These methods handle the @@ -124,12 +124,14 @@ async def shutdown(self) -> None: # be overridden. async def run(self) -> None: - """The core business logic, run in a background task. + """Execute the core business logic. Calls `startup`, and then loops calling `execute` followed by `idle`, tracking failures by watching for exceptions and updating ``success_count`` and ``failure_count``. When told to stop, calls `shutdown` followed by `close`. + + This method is normally run in a background task. """ self.logger.info("Starting up...") try: @@ -162,7 +164,7 @@ async def run(self) -> None: self.control.task_done() async def run_once(self) -> None: - """The core business logic, run only once. + """Execute the core business logic, only once. Calls `startup`, `execute`, `shutdown`, and `close`. """ @@ -176,7 +178,7 @@ async def run_once(self) -> None: await self.close() async def idle(self) -> None: - """The idle pause at the end of each loop.""" + """Pause at the end of each business loop.""" self.logger.info("Idling...") with self.timings.start("idle"): await self.pause(self.options.idle_time) @@ -302,7 +304,10 @@ def dump(self) -> BusinessData: ) async def _pause_no_return(self, seconds: float) -> None: - """Same as `pause` but returns `None`. + """Pause for up to the number of seconds, handling commands. + + The same as `pause`, but returns `None`, needed for proper typing of + `iter_with_timeout`. Parameters ---------- diff --git a/src/mobu/services/business/nublado.py b/src/mobu/services/business/nublado.py index 26718bd4..fc574372 100644 --- a/src/mobu/services/business/nublado.py +++ b/src/mobu/services/business/nublado.py @@ -113,7 +113,7 @@ def __init__( @abstractmethod async def execute_code(self, session: JupyterLabSession) -> None: - """The core of the execution loop. + """Execute some code inside the Jupyter lab. Must be overridden by subclasses to use the provided lab session to perform whatever operations are desired inside the lab. If multiple @@ -158,7 +158,6 @@ async def startup(self) -> None: self.logger.warning(msg) async def execute(self) -> None: - """The work done in each iteration of the loop.""" if self.options.delete_lab or await self._client.is_lab_stopped(): self._image = None if not await self.spawn_lab(): @@ -171,7 +170,7 @@ async def execute(self) -> None: await self.delete_lab() async def execution_idle(self) -> bool: - """Executed between each unit of work execution. + """Pause between each unit of work execution. This is not used directly by `NubladoBusiness`. It should be called by subclasses in `execute_code` in between each block of code that is @@ -184,7 +183,6 @@ async def shutdown(self) -> None: await self.delete_lab() async def idle(self) -> None: - """The idle pause at the end of each loop.""" if self.options.jitter: self.logger.info("Idling...") with self.timings.start("idle"): diff --git a/src/mobu/services/flock.py b/src/mobu/services/flock.py index 2e85ebc4..7f04f233 100644 --- a/src/mobu/services/flock.py +++ b/src/mobu/services/flock.py @@ -12,7 +12,7 @@ from safir.datetime import current_datetime from structlog.stdlib import BoundLogger -from ..exceptions import MonkeyNotFoundException +from ..exceptions import MonkeyNotFoundError from ..models.flock import FlockConfig, FlockData, FlockSummary from ..models.user import AuthenticatedUser, User, UserSpec from .monkey import Monkey @@ -60,10 +60,26 @@ def dump(self) -> FlockData: ) def get_monkey(self, name: str) -> Monkey: - """Retrieve a given monkey by name.""" + """Retrieve a given monkey by name. + + Parameters + ---------- + name + Name of monkey to return. + + Returns + ------- + Monkey + Requested monkey. + + Raises + ------ + MonkeyNotFoundError + Raised if no monkey was found with that name. + """ monkey = self._monkeys.get(name) if not monkey: - raise MonkeyNotFoundException(name) + raise MonkeyNotFoundError(name) return monkey def list_monkeys(self) -> list[str]: diff --git a/src/mobu/services/manager.py b/src/mobu/services/manager.py index 2ee871c2..7d722ef4 100644 --- a/src/mobu/services/manager.py +++ b/src/mobu/services/manager.py @@ -10,7 +10,7 @@ from structlog.stdlib import BoundLogger from ..config import config -from ..exceptions import FlockNotFoundException +from ..exceptions import FlockNotFoundError from ..models.flock import FlockConfig, FlockSummary from .flock import Flock @@ -98,12 +98,12 @@ def get_flock(self, name: str) -> Flock: Raises ------ - FlockNotFoundException + FlockNotFoundError Raised if no flock was found with that name. """ flock = self._flocks.get(name) if flock is None: - raise FlockNotFoundException(name) + raise FlockNotFoundError(name) return flock def list_flocks(self) -> list[str]: @@ -133,9 +133,14 @@ async def stop_flock(self, name: str) -> None: ---------- name Name of flock to stop. + + Raises + ------ + FlockNotFoundError + Raised if no flock was found with that name. """ flock = self._flocks.get(name) if flock is None: - raise FlockNotFoundException(name) + raise FlockNotFoundError(name) del self._flocks[name] await flock.stop() diff --git a/src/monkeyflocker/cli.py b/src/monkeyflocker/cli.py index 7e5b6aea..44c4726a 100644 --- a/src/monkeyflocker/cli.py +++ b/src/monkeyflocker/cli.py @@ -13,10 +13,7 @@ @click.group(context_settings={"help_option_names": ["-h", "--help"]}) @click.version_option(message="%(version)s") def main() -> None: - """monkeyflocker main. - - Command-line interface to manage mobu monkeys. - """ + """Command-line interface to manage mobu monkeys.""" pass