Merge pull request #87 from lsst-sqre:tickets/DM-48307

DM-48307: Updates for new ruff, mypy, Pydantic and Sqlalchemy changes

.pre-commit-config.yaml

@@ -1,13 +1,13 @@
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.6.0
+    rev: v5.0.0
     hooks:
       - id: trailing-whitespace
       - id: check-yaml
       - id: check-toml
 
   - repo: https://github.com/charliermarsh/ruff-pre-commit
-    rev: v0.6.4
+    rev: v0.8.6
     hooks:
       - id: ruff
         args: [--fix, --exit-non-zero-on-fix]

Makefile

@@ -29,20 +29,9 @@ update-deps:
 	pip install --upgrade uv
 	uv pip install --upgrade pre-commit
 	pre-commit autoupdate
-	uv pip compile --upgrade --generate-hashes			\
+	uv pip compile --upgrade --universal --generate-hashes		\
 	    --output-file requirements/main.txt requirements/main.in
-	uv pip compile --upgrade --generate-hashes			\
+	uv pip compile --upgrade --universal --generate-hashes		\
 	    --output-file requirements/dev.txt requirements/dev.in
-	uv pip compile --upgrade --generate-hashes			\
-	    --output-file requirements/tox.txt requirements/tox.in
-
-# Useful for testing against a Git version of Safir.
-.PHONY: update-deps-no-hashes
-update-deps-no-hashes:
-	pip install --upgrade uv
-	uv pip compile --upgrade					\
-	    --output-file requirements/main.txt requirements/main.in
-	uv pip compile --upgrade					\
-	    --output-file requirements/dev.txt requirements/dev.in
-	uv pip compile --upgrade					\
+	uv pip compile --upgrade --universal --generate-hashes		\
 	    --output-file requirements/tox.txt requirements/tox.in

changelog.d/20250106_164329_jsick_DM_48307.md

@@ -0,0 +1,20 @@
+<!-- Delete the sections that don't apply -->
+
+### Backwards-incompatible changes
+
+-
+
+### New features
+
+-
+
+### Bug fixes
+
+-
+
+### Other changes
+
+- Update the `make update` command in the Makefile to use the `--universal` flag with `uv pip compile`.
+- Begin SQLAlchemy 2 migration by adopting the new `DeclarativeBase`, `mapped_column`, and the `Mapped` type.
+- Fix type checking issues for Pydantic fields to use empty lists as the default, rather than using a default factory.
+- Explicitly set `asyncio_default_fixture_loop_scope` to function. An explicit setting is now required by pytest-asyncio.

pyproject.toml

@@ -57,6 +57,7 @@ exclude_lines = [
 ]
 
 [tool.pytest.ini_options]
+asyncio_default_fixture_loop_scope = "function"
 asyncio_mode = "strict"
 python_files = [
     "tests/*.py",

requirements/dev.in

@@ -23,3 +23,4 @@ respx
 types-PyYAML
 documenteer[guide]>=1.0
 httpx-sse == 0.4.0
+scriv

src/timessquare/config.py

@@ -12,7 +12,7 @@
 from safir.logging import LogLevel, Profile
 from safir.pydantic import EnvAsyncPostgresDsn, EnvRedisDsn
 
-__all__ = ["Config", "Profile", "LogLevel"]
+__all__ = ["Config", "LogLevel", "Profile"]
 
 
 class Config(BaseSettings):

src/timessquare/dbschema/base.py

@@ -2,8 +2,10 @@
 
 from __future__ import annotations
 
-from sqlalchemy.orm import declarative_base
+from sqlalchemy.orm import DeclarativeBase
 
 __all__ = ["Base"]
 
-Base = declarative_base()
+
+class Base(DeclarativeBase):
+    """The base class for all SQLAlchemy table schemas."""

src/timessquare/dbschema/page.py

@@ -6,7 +6,8 @@
 from datetime import datetime
 from typing import Any
 
-from sqlalchemy import JSON, Column, DateTime, Integer, Unicode, UnicodeText
+from sqlalchemy import JSON, DateTime, Integer, Unicode, UnicodeText
+from sqlalchemy.orm import Mapped, mapped_column
 
 from .base import Base
 
@@ -31,73 +32,77 @@ class SqlPage(Base):
 
     __tablename__ = "pages"
 
-    name: str = Column(
+    name: Mapped[str] = mapped_column(
         Unicode(32), primary_key=True, default=lambda: uuid.uuid4().hex
     )
     """The primary key, and also the ID for the Page REST API."""
 
-    ipynb: str = Column(UnicodeText, nullable=False)
+    ipynb: Mapped[str] = mapped_column(UnicodeText, nullable=False)
     """The Jinja-parameterized notebook, as a JSON-formatted string."""
 
-    parameters: dict[str, Any] = Column(JSON, nullable=False)
+    parameters: Mapped[dict[str, Any]] = mapped_column(JSON, nullable=False)
     """Parameters and their jsonschema descriptors."""
 
-    title: str = Column(UnicodeText, nullable=False)
+    title: Mapped[str] = mapped_column(UnicodeText, nullable=False)
     """Display title of the notebook."""
 
-    date_added: datetime = Column(DateTime, nullable=False)
+    date_added: Mapped[datetime] = mapped_column(DateTime, nullable=False)
     """Date when the page is registered through the Times Square API."""
 
-    authors: list[dict[str, Any]] = Column(JSON, nullable=False)
+    authors: Mapped[list[dict[str, Any]]] = mapped_column(JSON, nullable=False)
     """Authors of the notebook.
 
     The schema for this column is described by the NotebookSidecarFile
     authors field schema.
     """
 
-    tags: list[str] = Column(JSON, nullable=False)
+    tags: Mapped[list[str]] = mapped_column(JSON, nullable=False)
     """Tags (keywords) assigned to this page."""
 
-    uploader_username: str | None = Column(Unicode(64), nullable=True)
+    uploader_username: Mapped[str | None] = mapped_column(
+        Unicode(64), nullable=True
+    )
     """Username of the uploader, if this page is uploaded without GitHub
     backing.
     """
 
-    date_deleted: datetime | None = Column(DateTime)
+    date_deleted: Mapped[datetime | None] = mapped_column(DateTime)
     """A nullable datetime that is set to the datetime when the page is
     soft-deleted.
     """
 
-    description: str | None = Column(UnicodeText)
+    description: Mapped[str | None] = mapped_column(UnicodeText)
     """Description of a page (markdown-formatted)."""
 
-    cache_ttl: int | None = Column(Integer)
+    cache_ttl: Mapped[int | None] = mapped_column(Integer)
     """The cache TTL (seconds) for HTML renders, or None to retain renders
     indefinitely.
     """
 
-    github_owner: str | None = Column(Unicode(255))
+    github_owner: Mapped[str | None] = mapped_column(Unicode(255))
     """The GitHub repository owner (username or organization name) for
     GitHub-backed pages.
     """
 
-    github_repo: str | None = Column(Unicode(255))
+    github_repo: Mapped[str | None] = mapped_column(Unicode(255))
     """The GitHub repository name for GitHub-backed pages."""
 
-    github_commit: str | None = Column(Unicode(40))
+    github_commit: Mapped[str | None] = mapped_column(Unicode(40))
     """The SHA of the commit this page corresponds to; only used for pages
     associated with a GitHub Check Run.
     """
 
-    repository_path_prefix: str | None = Column(Unicode(2048))
+    repository_path_prefix: Mapped[str | None] = mapped_column(Unicode(2048))
     """The repository path prefix, relative to the root of the directory."""
 
-    repository_display_path_prefix: str | None = Column(Unicode(2048))
+    repository_display_path_prefix: Mapped[str | None] = mapped_column(
+        Unicode(2048)
+    )
     """The repository path prefix, relative to the configured root of Times
     Square notebooks in a repository.
     """
 
-    repository_path_stem: str | None = Column(Unicode(255))
+    repository_path_stem: Mapped[str | None] = mapped_column(Unicode(255))
     """The filename stem (without prefix and without extension) of the
     source file in the GitHub repository for GitHub-backed pages.
 
@@ -106,22 +111,26 @@ class SqlPage(Base):
     the corresponding files.
     """
 
-    repository_source_extension: str | None = Column(Unicode(255))
+    repository_source_extension: Mapped[str | None] = mapped_column(
+        Unicode(255)
+    )
     """The filename extension of the source file in the GitHub
     repository for GitHub-backed pages.
 
     Combine with repository_path_stem to get the file path.
     """
 
-    repository_sidecar_extension: str | None = Column(Unicode(255))
+    repository_sidecar_extension: Mapped[str | None] = mapped_column(
+        Unicode(255)
+    )
     """The filename extension of the sidecar YAML file in the GitHub
     repository for GitHub-backed pages.
 
     Combine with repository_path_stem to get the file path.
     """
 
-    repository_source_sha: str | None = Column(Unicode(40))
+    repository_source_sha: Mapped[str | None] = mapped_column(Unicode(40))
     """The git tree sha of the source file for GitHub-backed pages."""
 
-    repository_sidecar_sha: str | None = Column(Unicode(40))
+    repository_sidecar_sha: Mapped[str | None] = mapped_column(Unicode(40))
     """The git tree sha of the sidecar YAML file for GitHub-backed pages."""

src/timessquare/handlers/external/endpoints.py

@@ -23,7 +23,7 @@
 from .githubwebhooks import router as webhook_router
 from .models import Index
 
-__all__ = ["get_index", "external_router", "post_github_webhook"]
+__all__ = ["external_router", "get_index", "post_github_webhook"]
 
 external_router = APIRouter()
 """FastAPI router for all external handlers."""

src/timessquare/handlers/external/githubwebhooks.py

@@ -25,20 +25,20 @@
 from timessquare.config import config
 
 __all__ = [
-    "router",
-    "handle_installation_created",
-    "handle_installation_unsuspend",
-    "handle_installation_deleted",
-    "handle_installation_suspend",
-    "handle_repositories_added",
-    "handle_repositories_removed",
     "handle_check_run_created",
     "handle_check_run_rerequested",
     "handle_check_suite_request",
+    "handle_installation_created",
+    "handle_installation_deleted",
+    "handle_installation_suspend",
+    "handle_installation_unsuspend",
+    "handle_ping",
     "handle_pr_opened",
     "handle_pr_sync",
     "handle_push_event",
-    "handle_ping",
+    "handle_repositories_added",
+    "handle_repositories_removed",
+    "router",
 ]
 
 

src/timessquare/handlers/internal.py

@@ -26,7 +26,6 @@
         " a health check. This route is not exposed outside the cluster and"
         " therefore cannot be used by external clients."
     ),
-    response_model=Metadata,
     response_model_exclude_none=True,
     summary="Application metadata",
     include_in_schema=False,

src/timessquare/handlers/v1/endpoints.py

@@ -82,7 +82,6 @@
 
 @v1_router.get(
     "/",
-    response_model=Index,
     summary="v1 API metadata",
 )
 async def get_index(
@@ -104,7 +103,6 @@ async def get_index(
 
 @v1_router.get(
     "/pages/{page}",
-    response_model=Page,
     summary="Page metadata",
     name="get_page",
     tags=[ApiTags.pages],
@@ -136,7 +134,6 @@ async def get_page(
 
 @v1_router.get(
     "/pages",
-    response_model=list[PageSummary],
     summary="List pages",
     name="get_pages",
     tags=[ApiTags.pages],
@@ -156,7 +153,6 @@ async def get_pages(
 
 @v1_router.post(
     "/pages",
-    response_model=Page,
     summary="Create a new page",
     status_code=201,
     tags=[ApiTags.pages],
@@ -378,7 +374,6 @@ async def get_page_html(
     "/pages/{page}/html",
     summary="Delete the cached HTML of a notebook.",
     name="delete_page_html",
-    response_model=DeleteHtmlResponse,
     tags=[ApiTags.pages],
     responses={
         404: {"description": "Cached HTML not found", "model": ErrorModel},
@@ -423,7 +418,6 @@ async def delete_page_html(
     "/pages/{page}/htmlstatus",
     summary="Get the status of a page's HTML rendering",
     name="get_page_html_status",
-    response_model=HtmlStatus,
     tags=[ApiTags.pages],
     responses={
         404: {"description": "Page not found", "model": ErrorModel},
@@ -498,7 +492,6 @@ async def get_page_html_events(
     "/github",
     summary="Get a tree of GitHub-backed pages",
     name="get_github_tree",
-    response_model=GitHubContentsRoot,
     tags=[ApiTags.github],
 )
 async def get_github_tree(
@@ -519,7 +512,6 @@ async def get_github_tree(
 
 @v1_router.get(
     "/github/{display_path:path}",
-    response_model=Page,
     summary="Metadata for GitHub-backed page",
     name="get_github_page",
     tags=[ApiTags.github],
@@ -556,7 +548,6 @@ async def get_github_page(
     "/github-pr/{owner}/{repo}/{commit}",
     summary="Get a tree of GitHub PR preview pages",
     name="get_github_pr_tree",
-    response_model=GitHubPrContents,
     tags=[ApiTags.pr],
 )
 async def get_github_pr_tree(
@@ -607,7 +598,6 @@ async def get_github_pr_tree(
 
 @v1_router.get(
     "/github-pr/{owner}/{repo}/{commit}/{path:path}",
-    response_model=Page,
     summary="Metadata for page in a pull request",
     name="get_github_pr_page",
     tags=[ApiTags.pr],

src/timessquare/handlers/v1/models.py

@@ -73,13 +73,13 @@ class Index(BaseModel):
     description="Date when the page was originally added.",
 )
 
-page_authors_field = Field(
-    default_factory=list,
+page_authors_field: list[Person] = Field(
+    default=[],
     title="Page authors",
     description="Authors of the page",
 )
 
-page_tags_field = Field(default_factory=list, title="Tags (keywords)")
+page_tags_field: list[str] = Field(default=[], title="Tags (keywords)")
 
 page_url_field = Field(
     ...,

src/timessquare/worker/functions/__init__.py

@@ -9,13 +9,13 @@
 from .repo_removed import repo_removed
 
 __all__ = [
-    "ping",
-    "repo_push",
-    "repo_added",
-    "repo_removed",
-    "pull_request_sync",
     "compute_check_run",
     "create_check_run",
     "create_rerequested_check_run",
+    "ping",
+    "pull_request_sync",
     "replace_nbhtml",
+    "repo_added",
+    "repo_push",
+    "repo_removed",
 ]