From 3bc694384ab45a200cf3629e6357fda9b13ed12d Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Wed, 24 Jan 2024 15:42:15 -0800 Subject: [PATCH] Use Ruff and enforce full type annotation coverage (#60) I got here because I was trying to get rid of some `no-untyped-call` mypy errors internally, which were because our `__init__` method doesn't have annotations. To fix it thoroughly: - Use Ruff to enforce full annotation coverage - Also replace other linting tools (pyupgrade, pycln, isort, flake8) so we have fewer tools to worry about - Update the project URL because I noticed it - Fix any new errors --- .pre-commit-config.yaml | 30 ++++--------------------- pyproject.toml | 46 +++++++++++++++++++++++++++++++++------ src/fastapi_poe/base.py | 22 ++++++++++++------- src/fastapi_poe/client.py | 6 ++--- tests/test_types.py | 3 +-- 5 files changed, 61 insertions(+), 46 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index c9d4387..620295c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,22 +1,9 @@ repos: - # Order matters because pyupgrade may make changes that pycln and black have to clean up - - repo: https://github.com/asottile/pyupgrade - rev: v3.8.0 + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.1.14 hooks: - - id: pyupgrade - args: [--py37-plus] - - - repo: https://github.com/hadialqattan/pycln - rev: v2.1.5 - hooks: - - id: pycln - args: [--config=pyproject.toml] - - - repo: https://github.com/pycqa/isort - rev: 5.12.0 - hooks: - - id: isort - name: isort (python) + - id: ruff + args: [--fix] - repo: https://github.com/psf/black rev: 23.3.0 @@ -30,15 +17,6 @@ repos: - id: end-of-file-fixer - id: trailing-whitespace - - repo: https://github.com/pycqa/flake8 - rev: 6.0.0 - hooks: - - id: flake8 - additional_dependencies: - - flake8-bugbear - - flake8-comprehensions - - flake8-simplify - - repo: https://github.com/pre-commit/mirrors-prettier rev: v3.0.0-alpha.9-for-vscode hooks: diff --git a/pyproject.toml b/pyproject.toml index 17dca17..8950946 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -29,7 +29,7 @@ dependencies = [ ] [project.urls] -"Homepage" = "https://github.com/quora/poe-protocol" +"Homepage" = "https://developer.poe.com/" [tool.pyright] pythonVersion = "3.7" @@ -38,10 +38,42 @@ pythonVersion = "3.7" target-version = ['py37'] skip-magic-trailing-comma = true -[tool.pycln] -all = true +[tool.ruff] +select = [ + "F", + "E", + "I", # import sorting + "ANN", # type annotations for everything + "C4", # flake8-comprehensions + "B", # bugbear + "SIM", # simplify + "UP", # pyupgrade + "PIE810", # startswith/endswith with a tuple + "SIM101", # mergeable isinstance() calls + "SIM201", # "not ... == ..." -> "... != ..." + "SIM202", # "not ... != ..." -> "... == ..." + "C400", # unnecessary list() calls + "C401", # unnecessary set() calls + "C402", # unnecessary dict() calls + "C403", # unnecessary listcomp within set() call + "C404", # unnecessary listcomp within dict() call + "C405", # use set literals + "C406", # use dict literals + "C409", # tuple() calls that can be replaced with literals + "C410", # list() calls that can be replaced with literals + "C411", # list() calls with genexps that can be replaced with listcomps + "C413", # unnecessary list() calls around sorted() + "C414", # unnecessary list() calls inside sorted() + "C417", # unnecessary map() calls that can be replaced with listcomps/genexps + "C418", # unnecessary dict() calls that can be replaced with literals + "PERF101", # unnecessary list() calls that can be replaced with literals +] + +ignore = [ + "B008", # do not perform function calls in argument defaults + "ANN101", # missing type annotation for self in method + "ANN102", # missing type annotation for cls in classmethod +] -[tool.isort] -profile = "black" -combine_as_imports = true -skip_gitignore = true +line-length = 100 +target-version = "py37" diff --git a/src/fastapi_poe/base.py b/src/fastapi_poe/base.py index f296f12..96f4f2b 100644 --- a/src/fastapi_poe/base.py +++ b/src/fastapi_poe/base.py @@ -6,7 +6,7 @@ import os import sys import warnings -from typing import AsyncIterable, BinaryIO, Dict, Optional, Union +from typing import AsyncIterable, Awaitable, BinaryIO, Callable, Dict, Optional, Union import httpx from fastapi import Depends, FastAPI, HTTPException, Request, Response @@ -15,6 +15,7 @@ from fastapi.security import HTTPAuthorizationCredentials, HTTPBearer from sse_starlette.sse import EventSourceResponse, ServerSentEvent from starlette.middleware.base import BaseHTTPMiddleware +from starlette.types import Message from fastapi_poe.types import ( AttachmentUploadResponse, @@ -43,15 +44,17 @@ class AttachmentUploadError(Exception): class LoggingMiddleware(BaseHTTPMiddleware): - async def set_body(self, request: Request): + async def set_body(self, request: Request) -> None: receive_ = await request._receive() - async def receive(): + async def receive() -> Message: return receive_ request._receive = receive - async def dispatch(self, request: Request, call_next): + async def dispatch( + self, request: Request, call_next: Callable[[Request], Awaitable[Response]] + ) -> Response: logger.info(f"Request: {request.method} {request.url}") try: # Per https://github.com/tiangolo/fastapi/issues/394#issuecomment-927272627 @@ -75,8 +78,9 @@ async def dispatch(self, request: Request, call_next): return response -async def http_exception_handler(request, ex): +async def http_exception_handler(request: Request, ex: Exception) -> Response: logger.error(ex) + return Response(status_code=500, content="Internal server error") http_bearer = HTTPBearer() @@ -139,7 +143,7 @@ async def on_error(self, error_request: ReportErrorRequest) -> None: logger.error(f"Error from Poe server: {error_request}") # Helpers for generating responses - def __init__(self): + def __init__(self) -> None: self._pending_file_attachment_tasks = {} async def post_message_attachment( @@ -232,7 +236,9 @@ async def _make_file_attachment_request( logger.error("An HTTP error occurred when attempting to attach file") raise - async def _process_pending_attachment_requests(self, message_id): + async def _process_pending_attachment_requests( + self, message_id: Identifier + ) -> None: try: await asyncio.gather( *self._pending_file_attachment_tasks.pop(message_id, []) @@ -434,7 +440,7 @@ async def index() -> Response: ) @app.post("/") - async def poe_post(request: Request, dict=Depends(auth_user)) -> Response: + async def poe_post(request: Request, dict: object = Depends(auth_user)) -> Response: request_body = await request.json() request_body["http_request"] = request if request_body["type"] == "query": diff --git a/src/fastapi_poe/client.py b/src/fastapi_poe/client.py index ae6b9ce..65375bc 100644 --- a/src/fastapi_poe/client.py +++ b/src/fastapi_poe/client.py @@ -17,8 +17,6 @@ from .types import ( ContentType, Identifier, - MetaResponse as MetaMessage, - PartialResponse as BotMessage, ProtocolMessage, QueryRequest, SettingsResponse, @@ -26,6 +24,8 @@ ToolDefinition, ToolResultDefinition, ) +from .types import MetaResponse as MetaMessage +from .types import PartialResponse as BotMessage PROTOCOL_VERSION = "1.0" MESSAGE_LENGTH_LIMIT = 10_000 @@ -274,7 +274,7 @@ async def _load_json_dict( {"data": data, "message_id": message_id}, ) # If they are returning invalid JSON, retrying immediately probably won't help - raise BotErrorNoRetry(f"Invalid JSON in {context!r} event") + raise BotErrorNoRetry(f"Invalid JSON in {context!r} event") from None if not isinstance(parsed, dict): await self.report_error( f"Expected JSON dict in {context!r} event", diff --git a/tests/test_types.py b/tests/test_types.py index 2f1eadf..d2e820c 100644 --- a/tests/test_types.py +++ b/tests/test_types.py @@ -1,10 +1,9 @@ import pydantic import pytest - from fastapi_poe.types import PartialResponse -def test_extra_attrs(): +def test_extra_attrs() -> None: with pytest.raises(pydantic.ValidationError): PartialResponse(text="hi", replaceResponse=True) # type: ignore