Merge pull request #245 from lsst-sqre/tickets/DM-38425

DM-38425: Adopt current Safir conventions

.pre-commit-config.yaml

@@ -2,22 +2,36 @@ repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
     rev: v4.4.0
     hooks:
+      - id: trailing-whitespace
       - id: check-yaml
       - id: check-toml
 
   - repo: https://github.com/PyCQA/isort
     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/

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

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`.

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.

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 = [

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",

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."""
 

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."""

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(
     "/",

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",
-                }
-            ]
-        },
-    )

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,20 +116,22 @@ 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
     # complex state logic of looping and handling a stop signal and should not
     # 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
         ----------