diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index c312dcb..7ef3a92 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -2,15 +2,16 @@ repos: - repo: https://github.com/pre-commit/pre-commit-hooks rev: v4.4.0 hooks: - - id: trailing-whitespace - - id: check-yaml + - id: check-merge-conflict - id: check-toml + - id: check-yaml + - id: trailing-whitespace - - repo: https://github.com/PyCQA/isort - rev: 5.12.0 + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.0.286 hooks: - - id: isort - additional_dependencies: [toml] + - id: ruff + args: [--fix, --exit-non-zero-on-fix] - repo: https://github.com/psf/black rev: 23.3.0 @@ -23,15 +24,3 @@ repos: - 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/pyproject.toml b/pyproject.toml index f9c0694..5460040 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -10,7 +10,7 @@ keywords = [ ] # https://pypi.org/classifiers/ classifiers = [ - "Development Status :: 4 - Beta", + "Development Status :: 5 - Production/Stable", "License :: OSI Approved :: MIT License", "Programming Language :: Python", "Programming Language :: Python :: 3", @@ -78,27 +78,12 @@ exclude = ''' # Use single-quoted strings so TOML treats the string like a Python r-string # Multi-line strings are implicitly treated by black as regular expressions -[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 (needed for docstring inheritance) - "D100", # Missing docstring in public module - # Properties shouldn't be written in imperative mode. This will be fixed - # post 6.1.1, see https://github.com/PyCQA/pydocstyle/pull/546 - "D401", -] - -[tool.isort] -profile = "black" -line_length = 79 -known_first_party = ["rsp_restspawner", "tests"] -skip = ["docs/conf.py"] - [tool.pytest.ini_options] asyncio_mode = "strict" +filterwarnings = [ + # Will probably be fixed with JupyterHub v4. + "ignore:'pipes' is deprecated:DeprecationWarning:jupyterhub.spawner" +] # The python_files setting is not for test detection (pytest will pick up any # test files named *_test.py without this setting) but to enable special # assert processing in any non-test supporting files under tests. We @@ -123,3 +108,107 @@ strict_equality = true warn_redundant_casts = true warn_unreachable = true warn_unused_ignores = 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] +exclude = [ + "docs/**", +] +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 + "ARG005", # unused lambda arguments are often legitimate + "BLE001", # we want to catch and report Exception in background tasks + "C414", # nested sorted is how you sort by multiple keys with reverse + "COM812", # omitting trailing commas allows black autoreformatting + "D102", # sometimes we use docstring inheritence + "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 + "D205", # our documentation style allows a folded first line + "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 + "FIX002", # point of a TODO comment is that we're not ready to fix it + "G004", # forbidding logging f-strings is appealing, but not our style + "RET505", # disagree that omitting else always makes code more readable + "PLR0913", # factory pattern uses constructors with many arguments + "PLR2004", # too aggressive about magic values + "PLW0603", # yes global is discouraged but if needed, it's needed + "S105", # good idea but too many false positives on non-passwords + "S106", # good idea but too many false positives on non-passwords + "S603", # not going to manually mark every subprocess call as reviewed + "S607", # using PATH is not a security vulnerability + "SIM102", # sometimes the formatting of nested if statements is clearer + "SIM117", # sometimes nested with contexts are 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 + "TD003", # we don't require issues be created for TODOs + "TID252", # if we're going to use relative imports, use them always + "TRY003", # good general advice but lint is way too aggressive + "TRY301", # sometimes raising exceptions inside try is the best flow +] +select = ["ALL"] +target-version = "py310" + +[tool.ruff.per-file-ignores] +"tests/**" = [ + "C901", # tests are allowed to be complex, sometimes that's convenient + "D101", # tests don't need docstrings + "D103", # tests don't need docstrings + "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 + "SLF001", # tests are allowed to access private members +] + +[tool.ruff.isort] +known-first-party = ["rsp_restspawner", "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 methods to allow the conflict with the +# built-in to rule out their use. +[tool.ruff.flake8-builtins] +builtins-ignorelist = [ + "all", + "any", + "dict", + "help", + "id", + "list", + "open", + "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" diff --git a/src/rsp_restspawner/auth.py b/src/rsp_restspawner/auth.py index 4660b34..eaabc23 100644 --- a/src/rsp_restspawner/auth.py +++ b/src/rsp_restspawner/auth.py @@ -2,7 +2,7 @@ from __future__ import annotations -from typing import Any, Optional +from typing import Any from jupyterhub.app import JupyterHub from jupyterhub.auth import Authenticator @@ -102,7 +102,7 @@ async def authenticate( This is not used in our authentication scheme. """ - raise NotImplementedError() + raise NotImplementedError def get_handlers(self, app: JupyterHub) -> list[Route]: """Register the header-only login and the logout handlers.""" @@ -121,7 +121,7 @@ def login_url(self, base_url: str) -> str: return url_path_join(base_url, "gafaelfawr/login") async def refresh_user( - self, user: User, handler: Optional[RequestHandler] = None + self, user: User, handler: RequestHandler | None = None ) -> bool | AuthInfo: """Optionally refresh the user's token.""" # If running outside of a Tornado handler, we can't refresh the auth diff --git a/src/rsp_restspawner/exceptions.py b/src/rsp_restspawner/exceptions.py index e438d33..44f586e 100644 --- a/src/rsp_restspawner/exceptions.py +++ b/src/rsp_restspawner/exceptions.py @@ -7,8 +7,6 @@ from __future__ import annotations -from typing import Optional - from httpx import HTTPError, HTTPStatusError, RequestError __all__ = [ @@ -65,7 +63,7 @@ def from_exception(cls, exc: HTTPError) -> ControllerWebError: body=exc.response.text, ) else: - message = f"{type(exc).__name__}: {str(exc)}" + message = f"{type(exc).__name__}: {exc!s}" if isinstance(exc, RequestError): return cls( message, @@ -79,11 +77,11 @@ def __init__( self, message: str, *, - method: Optional[str] = None, - url: Optional[str] = None, - status: Optional[int] = None, - reason: Optional[str] = None, - body: Optional[str] = None, + method: str | None = None, + url: str | None = None, + status: int | None = None, + reason: str | None = None, + body: str | None = None, ) -> None: self.message = message self.method = method diff --git a/src/rsp_restspawner/spawner.py b/src/rsp_restspawner/spawner.py index 2e634d9..d0c8abd 100644 --- a/src/rsp_restspawner/spawner.py +++ b/src/rsp_restspawner/spawner.py @@ -9,7 +9,7 @@ from enum import Enum from functools import wraps from pathlib import Path -from typing import Any, Concatenate, Optional, ParamSpec, TypeVar +from typing import Any, Concatenate, ParamSpec, TypeVar from httpx import AsyncClient, HTTPError, Response from httpx_sse import ServerSentEvent, aconnect_sse @@ -31,7 +31,7 @@ "RSPRestSpawner", ] -_CLIENT: Optional[AsyncClient] = None +_CLIENT: AsyncClient | None = None """Cached global HTTP client so that we can share a connection pool.""" @@ -195,7 +195,7 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: # Holds the future representing a spawn in progress, used by the # progress method to know when th spawn is finished and it should # exit. - self._start_future: Optional[asyncio.Task[str]] = None + self._start_future: asyncio.Task[str] | None = None @property def _client(self) -> AsyncClient: @@ -274,7 +274,7 @@ async def options_form(self, spawner: Spawner) -> str: return r.text @_convert_exception - async def poll(self) -> Optional[int]: + async def poll(self) -> int | None: """Check if the pod is running. Returns diff --git a/tests/spawner_test.py b/tests/spawner_test.py index b88eb51..c28fcea 100644 --- a/tests/spawner_test.py +++ b/tests/spawner_test.py @@ -18,10 +18,7 @@ async def collect_progress( spawner: RSPRestSpawner, ) -> list[dict[str, int | str]]: """Gather progress from a spawner and return it as a list when done.""" - result = [] - async for message in spawner.progress(): - result.append(message) - return result + return [m async for m in spawner.progress()] @pytest.mark.asyncio diff --git a/tests/support/controller.py b/tests/support/controller.py index d37df7d..5502799 100644 --- a/tests/support/controller.py +++ b/tests/support/controller.py @@ -4,7 +4,7 @@ import asyncio from collections.abc import AsyncIterator -from datetime import datetime, timedelta +from datetime import datetime, timedelta, timezone import respx from httpx import AsyncByteStream, Request, Response @@ -34,7 +34,7 @@ class MockProgress(AsyncByteStream): """ def __init__( - self, user: str, delay: timedelta, fail_during_spawn: bool = False + self, user: str, delay: timedelta, *, fail_during_spawn: bool = False ) -> None: self._user = user self._delay = delay @@ -50,7 +50,7 @@ async def __aiter__(self) -> AsyncIterator[bytes]: # sse-starlette sends these ping events periodically to keep the # connection alive. We should just ignore them. yield b"event: ping\r\n" - yield b"data: " + str(datetime.utcnow()).encode() + b"\r\n" + yield b"data: " + str(datetime.now(tz=timezone.utc)).encode() + b"\r\n" yield b"\r\n" yield b"event: info\r\n" @@ -144,7 +144,9 @@ def events(self, request: Request, user: str) -> Response: self._check_authorization(request) if not self._lab_status.get(user): return Response(status_code=404) - stream = MockProgress(user, self.delay, self.fail_during_spawn) + stream = MockProgress( + user, self.delay, fail_during_spawn=self.fail_during_spawn + ) return Response( status_code=200, headers={"Content-Type": "text/event-stream"}, @@ -174,7 +176,7 @@ def status(self, request: Request, user: str) -> Response: ) def _check_authorization( - self, request: Request, admin: bool = False + self, request: Request, *, admin: bool = False ) -> None: authorization = request.headers["Authorization"] auth_type, token = authorization.split(None, 1)