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

DM-38425: Configure Ruff and fix things it found

Makefile

@@ -1,8 +1,12 @@
 .PHONY: update-deps
 update-deps:
 	pip install --upgrade pip-tools pip setuptools
-	pip-compile --upgrade --resolver=backtracking --build-isolation --generate-hashes --output-file requirements/main.txt requirements/main.in
-	pip-compile --upgrade --resolver=backtracking --build-isolation --generate-hashes --output-file requirements/dev.txt requirements/dev.in
+	pip-compile --upgrade --resolver=backtracking --build-isolation	\
+	    --generate-hashes --allow-unsafe				\
+	    --output-file requirements/main.txt requirements/main.in
+	pip-compile --upgrade --resolver=backtracking --build-isolation	\
+	    --generate-hashes --allow-unsafe				\
+	    --output-file requirements/dev.txt requirements/dev.in
 
 # Useful for testing against a Git version of Safir.
 .PHONY: update-deps-no-hashes

pyproject.toml

@@ -59,7 +59,7 @@ exclude_lines = [
 
 [tool.black]
 line-length = 79
-target-version = ['py311']
+target-version = ["py311"]
 exclude = '''
 /(
     \.eggs
@@ -129,6 +129,88 @@ init_typed = true
 warn_required_dynamic_aliases = true
 warn_untyped_fields = true
 
+# The rule used with Ruff configuration is to disable every lint that has
+# legitimate exceptions that are not dodgy code, rather than cluttering code
+# with noqa markers. This is therefore a reiatively relaxed configuration that
+# errs on the side of disabling legitimate lints.
+#
+# Reference for settings: https://beta.ruff.rs/docs/settings/
+# Reference for rules: https://beta.ruff.rs/docs/rules/
+[tool.ruff]
+line-length = 79
+ignore = [
+    "ANN101",  # self should not have a type annotation
+    "ANN102",  # cls should not have a type annotation
+    "ANN401",  # sometimes Any is the right type
+    "ARG001",  # unused function arguments are often legitimate
+    "ARG002",  # unused method arguments are often legitimate
+    "BLE001",  # we want to catch and report Exception in background tasks
+    "COM812",  # omitting trailing commas allows black autoreformatting
+    "D102",    # sometimes we use docstring inheritence
+    "D103",    # FastAPI handlers should not have docstrings
+    "D104",    # don't see the point of documenting every package
+    "D105",    # our style doesn't require docstrings for magic methods
+    "D106",    # Pydantic uses a nested Config class that doesn't warrant docs
+    "EM101",   # justification (duplicate string in traceback) is silly
+    "EM102",   # justification (duplicate string in traceback) is silly
+    "FBT003",  # positional booleans are normal for Pydantic field defaults
+    "G004",    # forbidding logging f-strings is appealing, but not our style
+    "RET505",  # disagree that omitting else always makes code more readable
+    "PLR2004", # too aggressive about magic values
+    "SIM102",  # sometimes the formatting of nested if statements is clearer
+    "TCH001",  # we decided to not maintain separate TYPE_CHECKING blocks
+    "TCH002",  # we decided to not maintain separate TYPE_CHECKING blocks
+    "TCH003",  # we decided to not maintain separate TYPE_CHECKING blocks
+    "TID252",  # if we're going to use relative imports, use them always
+    "TRY003",  # good general advice but lint is way too aggressive
+]
+select = ["ALL"]
+target-version = "py311"
+
+[tool.ruff.per-file-ignores]
+"tests/**" = [
+    "PLR0915", # tests are allowed to be long, sometimes that's convenient
+    "PT012",   # way too aggressive about limiting pytest.raises blocks
+    "S101",    # tests should use assert
+    "S106",    # tests are allowed to hard-code dummy passwords
+    "SLF001",  # tests are allowed to access private members
+]
+
+[tool.ruff.isort]
+known-first-party = ["monkeyflocker", "mobu", "tests"]
+split-on-trailing-comma = false
+
+[tool.ruff.flake8-bugbear]
+extend-immutable-calls = [
+    "fastapi.Form",
+    "fastapi.Header",
+    "fastapi.Depends",
+    "fastapi.Path",
+    "fastapi.Query",
+]
+
+# These are too useful as attributes or functions to allow the conflict with
+# the built-in to rule out their use.
+[tool.ruff.flake8-builtins]
+builtins-ignorelist = [
+    "help",
+    "id",
+    "type",
+]
+
+[tool.ruff.flake8-pytest-style]
+fixture-parentheses = false
+mark-parentheses = false
+
+[tool.ruff.pep8-naming]
+classmethod-decorators = [
+    "pydantic.root_validator",
+    "pydantic.validator",
+]
+
+[tool.ruff.pydocstyle]
+convention = "numpy"
+
 [tool.scriv]
 categories = [
     "Backwards-incompatible changes",

requirements/dev.in

@@ -18,6 +18,7 @@ pytest-cov
 pytest-httpx
 pytest-sugar
 respx
+ruff
 types-PyYAML
 types-requests
 

requirements/dev.txt

@@ -2,7 +2,7 @@
 # This file is autogenerated by pip-compile with Python 3.11
 # by the following command:
 #
-#    pip-compile --generate-hashes --output-file=requirements/dev.txt --resolver=backtracking requirements/dev.in
+#    pip-compile --allow-unsafe --generate-hashes --output-file=requirements/dev.txt --resolver=backtracking requirements/dev.in
 #
 anyio==3.6.2 \
     --hash=sha256:25ea0d673ae30af41a0c442f81cf3b38c7e79fdc7b60335a4c14e05eb0947421 \
@@ -310,9 +310,9 @@ mypy-extensions==1.0.0 \
     --hash=sha256:4392f6c0eb8a5668a69e23d168ffa70f0be9ccfd32b5cc2d26a34ae5b844552d \
     --hash=sha256:75dbf8955dc00442a438fc4d0666508a9a97b6bd41aa2f0ffe9d2f2725af0782
     # via mypy
-nodeenv==1.7.0 \
-    --hash=sha256:27083a7b96a25f2f5e1d8cb4b6317ee8aeda3bdd121394e5ac54e498028a042e \
-    --hash=sha256:e0e7f7dfb85fc5394c6fe1e8fa98131a2473e04311a45afb6508f7cf1836fa2b
+nodeenv==1.8.0 \
+    --hash=sha256:d51e0c37e64fbf47d017feac3145cdbb58836d7eee8c6f6d3b6880c5456227d2 \
+    --hash=sha256:df865724bb3c3adc86b3876fa209771517b0cfe596beff01a92700e0e8be4cec
     # via pre-commit
 packaging==23.1 \
     --hash=sha256:994793af429502c4ea2ebf6bf664629d07c1a9fe974af92966e4b8d2df7edc61 \
@@ -321,9 +321,9 @@ packaging==23.1 \
     #   -c requirements/main.txt
     #   pytest
     #   pytest-sugar
-platformdirs==3.5.0 \
-    --hash=sha256:47692bc24c1958e8b0f13dd727307cff1db103fca36399f457da8e05f222fdc4 \
-    --hash=sha256:7954a68d0ba23558d753f73437c55f89027cf8f5108c19844d4b82e5af396335
+platformdirs==3.5.1 \
+    --hash=sha256:412dae91f52a6f84830f39a8078cecd0e866cb72294a5c66808e74d5e88d251f \
+    --hash=sha256:e2378146f1964972c03c085bb5662ae80b2b8c06226c54b2ff4aa9483e8a13a5
     # via virtualenv
 pluggy==1.0.0 \
     --hash=sha256:4224373bacce55f955a878bf9cfa763c1e360858e330072059e10bad68531159 \
@@ -416,6 +416,25 @@ respx==0.20.1 \
     --hash=sha256:372f06991c03d1f7f480a420a2199d01f1815b6ed5a802f4e4628043a93bd03e \
     --hash=sha256:cc47a86d7010806ab65abdcf3b634c56337a737bb5c4d74c19a0dfca83b3bc73
     # via -r requirements/dev.in
+ruff==0.0.266 \
+    --hash=sha256:1c9d52820a4744e9cfb4bc2492c8ecd6329147366a7e86cf198c0b2f7f4532af \
+    --hash=sha256:422da2da16fd1a38f9731c2f54320e2550a1ae51f9f494b6c11002acd8b53b43 \
+    --hash=sha256:5a80e6279d2af09d0827804356c6c73b0f47b9ee116dfd0a225aa2a203839a17 \
+    --hash=sha256:6722812b8a5e5ca57f20983dfa4eed4b379ce2c5648e8a4462c43dc90a6f7ab0 \
+    --hash=sha256:75b6c78a1b9d4eb4146a110949a9530f788438b813351bddcdcf373e46b33910 \
+    --hash=sha256:784cb211de2a67a1374e9936db9de8923c922e64efc5f4ddc37511dd464459a1 \
+    --hash=sha256:7ad7b69b40e009aa96551971bea740bf2332c1739085cf7a79fee6ef11105d29 \
+    --hash=sha256:86d931303fb5c43945ab5774d53a28167f4c9b40712553b52329f16a79e9ff10 \
+    --hash=sha256:8a80b5641572c76a77be69d063ee7b408b1aa9ef583b97862a1d8b2dfac41a15 \
+    --hash=sha256:8afe32e66ecd18f7f169587ea86260ce0666a026c83370dc50ba2a7eca8acd40 \
+    --hash=sha256:94304a0c65219e5156caa0846b2b9dd35eb508aa79f470a7a4a6363681a6ee0a \
+    --hash=sha256:a358e036c6c797c32986e2c6a1dc258a83439c8283c4520d029e505a4d2fd9a1 \
+    --hash=sha256:a53dc7d99937305519072e390714fab9d6ccffc57c1f04e81eb95d6549c333df \
+    --hash=sha256:c543bcf4d55f244816e00811dc3a00a122ba14cc0c0ba636dbaf3a765eaeba81 \
+    --hash=sha256:e9a89114336a2d2ad5fd237381dfbaf1d8e7e8b34f01eba283549e3e8802f6f8 \
+    --hash=sha256:fbc094bc88893e43b506343b7643f46d0ee58b8726cb0521cf72caa95730cfe1 \
+    --hash=sha256:ff892ed9108ee888c6a8f2b8efec47c40dd7dac1977d575e2b4d69ef9363af25
+    # via -r requirements/dev.in
 scriv[toml]==1.3.1 \
     --hash=sha256:4df597ee0a7b3dcc0d5315d7649df091efcb1b3ff08f0b51a698340727d17118 \
     --hash=sha256:9137e343f98dd65b8ba3dda286739cb21b0b04fa1fc4a25ef4a8342556686f6b
@@ -466,6 +485,8 @@ virtualenv==20.23.0 \
     --hash=sha256:a85caa554ced0c0afbd0d638e7e2d7b5f92d23478d05d17a76daeac8f279f924
     # via pre-commit
 
-# WARNING: The following packages were not pinned, but pip requires them to be
-# pinned when the requirements file includes hashes. Consider using the --allow-unsafe flag.
-# setuptools
+# The following packages are considered to be unsafe in a requirements file:
+setuptools==67.7.2 \
+    --hash=sha256:23aaf86b85ca52ceb801d32703f12d77517b2556af839621c641fca11287952b \
+    --hash=sha256:f104fa03692a2602fa0fec6c6a9e63b6c8a968de13e17c026957dd1f53d80990
+    # via nodeenv

requirements/main.txt

@@ -2,7 +2,7 @@
 # This file is autogenerated by pip-compile with Python 3.11
 # by the following command:
 #
-#    pip-compile --generate-hashes --output-file=requirements/main.txt --resolver=backtracking requirements/main.in
+#    pip-compile --allow-unsafe --generate-hashes --output-file=requirements/main.txt --resolver=backtracking requirements/main.in
 #
 aiojobs==1.1.0 \
     --hash=sha256:2080af76fda924bf2a60446f9b4435b11bb2418315c82664b07c2bb369b595d3 \

src/mobu/asyncio.py

@@ -3,6 +3,7 @@
 from __future__ import annotations
 
 import asyncio
+import contextlib
 from asyncio import Task
 from collections.abc import Awaitable, Callable, Coroutine
 from typing import TypeVar
@@ -40,12 +41,10 @@ async def wait_first(*args: Coroutine[None, None, T]) -> T | None:
         [asyncio.create_task(a) for a in args],
         return_when=asyncio.FIRST_COMPLETED,
     )
-    gather = asyncio.gather(*pending)
-    gather.cancel()
-    try:
+    with contextlib.suppress(asyncio.CancelledError):
+        gather = asyncio.gather(*pending)
+        gather.cancel()
         await gather
-    except asyncio.CancelledError:
-        pass
     try:
         return done.pop().result()
     except StopAsyncIteration:

src/mobu/dependencies/context.py

@@ -7,7 +7,6 @@
 """
 
 from dataclasses import dataclass
-from typing import Optional
 
 from fastapi import Depends, Request
 from safir.dependencies.gafaelfawr import auth_logger_dependency
@@ -51,7 +50,7 @@ class ContextDependency:
     """
 
     def __init__(self) -> None:
-        self._process_context: Optional[ProcessContext] = None
+        self._process_context: ProcessContext | None = None
 
     async def __call__(
         self,

src/mobu/exceptions.py

@@ -4,7 +4,7 @@
 
 import re
 from datetime import datetime
-from typing import Optional, Self
+from typing import Self
 
 from fastapi import status
 from pydantic import ValidationError
@@ -79,7 +79,7 @@ class GafaelfawrParseError(SlackException):
 
     @classmethod
     def from_exception(
-        cls, exc: ValidationError, user: Optional[str] = None
+        cls, exc: ValidationError, user: str | None = None
     ) -> Self:
         """Create an exception from a Pydantic parse failure.
 
@@ -99,7 +99,7 @@ def from_exception(
         return cls("Unable to parse reply from Gafalefawr", error, user)
 
     def __init__(
-        self, message: str, error: str, user: Optional[str] = None
+        self, message: str, error: str, user: str | None = None
     ) -> None:
         super().__init__(message, user)
         self.error = error
@@ -155,11 +155,11 @@ class MobuSlackException(SlackException):
     """
 
     def __init__(
-        self, msg: str, user: str, *, failed_at: Optional[datetime] = None
+        self, msg: str, user: str, *, failed_at: datetime | None = None
     ) -> None:
-        super().__init__(msg, user)
-        self.started_at: Optional[datetime] = None
-        self.event: Optional[str] = None
+        super().__init__(msg, user, failed_at=failed_at)
+        self.started_at: datetime | None = None
+        self.event: str | None = None
         self.annotations: dict[str, str] = {}
 
     def to_slack(self) -> SlackMessage:
@@ -260,10 +260,10 @@ def __init__(
         self,
         *,
         user: str,
-        code: Optional[str] = None,
+        code: str | None = None,
         code_type: str = "code",
-        error: Optional[str] = None,
-        status: Optional[str] = None,
+        error: str | None = None,
+        status: str | None = None,
     ) -> None:
         super().__init__("Code execution failed", user)
         self.code = code
@@ -323,10 +323,26 @@ class JupyterSpawnError(MobuSlackException):
 
     @classmethod
     def from_exception(cls, log: str, exc: Exception, user: str) -> Self:
+        """Convert from an arbitrary exception to a spawn error.
+
+        Parameters
+        ----------
+        log
+            Log of the spawn to this point.
+        exc
+            Exception that terminated the spawn attempt.
+        user
+            Username of the user spawning the lab.
+
+        Returns
+        -------
+        JupyterSpawnError
+            Converted exception.
+        """
         return cls(log, user, f"{type(exc).__name__}: {str(exc)}")
 
     def __init__(
-        self, log: str, user: str, message: Optional[str] = None
+        self, log: str, user: str, message: str | None = None
     ) -> None:
         if message:
             message = f"Spawning lab failed: {message}"
@@ -347,7 +363,7 @@ def to_slack(self) -> SlackMessage:
 class JupyterTimeoutError(MobuSlackException):
     """Timed out waiting for the lab to spawn."""
 
-    def __init__(self, msg: str, user: str, log: Optional[str] = None) -> None:
+    def __init__(self, msg: str, user: str, log: str | None = None) -> None:
         super().__init__(msg, user)
         self.log = log
 
@@ -399,10 +415,10 @@ def __init__(
         msg: str,
         *,
         user: str,
-        code: Optional[int] = None,
-        reason: Optional[str] = None,
-        status: Optional[int] = None,
-        body: Optional[bytes] = None,
+        code: int | None = None,
+        reason: str | None = None,
+        status: int | None = None,
+        body: bytes | None = None,
     ) -> None:
         super().__init__(msg, user)
         self.code = code

src/mobu/factory.py

@@ -2,8 +2,6 @@
 
 from __future__ import annotations
 
-from typing import Optional
-
 import structlog
 from httpx import AsyncClient
 from safir.slack.webhook import SlackWebhookClient
@@ -64,7 +62,7 @@ class Factory:
     """
 
     def __init__(
-        self, context: ProcessContext, logger: Optional[BoundLogger] = None
+        self, context: ProcessContext, logger: BoundLogger | None = None
     ) -> None:
         self._context = context
         self._logger = logger if logger else structlog.get_logger("mobu")

src/mobu/handlers/external.py

@@ -2,6 +2,7 @@
 
 import json
 from collections.abc import Iterator
+from pathlib import Path
 from typing import Any
 
 from fastapi import APIRouter, Depends, Response
@@ -176,7 +177,7 @@ def get_monkey_log(
     # Note that this is not async, so this handler must be sync so that
     # FastAPI will run it in a thread pool.
     def iterfile() -> Iterator[bytes]:
-        with open(logfile, "rb") as fh:
+        with Path(logfile).open("rb") as fh:
             yield from fh
 
     filename = f"{flock}-{monkey}-{current_datetime()}"