chore: Modernize Python tooling (ruff, uv, mypy)

.pre-commit-config.yaml

@@ -28,6 +28,10 @@ repos:
         language: system
         types: [python]
         require_serial: true
+        # Keep in sync with [tool.mypy] exclude in pyproject.toml. pre-commit
+        # passes filenames explicitly, which bypasses mypy's own `exclude`, so
+        # these dirs must be re-excluded here to honor the same policy.
+        exclude: '^(rust_snuba/|tests/datasets/|tests/query/|test_distributed_migrations/)'
 
       - id: validate-configs-syntax
         name: validate-configs-syntax

Makefile

@@ -37,6 +37,20 @@ test-distributed:
 
 tests: test
 
+lint:
+	uv run ruff check --fix .
+	uv run ruff format .
+.PHONY: lint
+
+lint-check:
+	uv run ruff check .
+	uv run ruff format --check .
+.PHONY: lint-check
+
+typecheck:
+	uv run mypy .
+.PHONY: typecheck
+
 api-tests:
 	SNUBA_SETTINGS=test pytest -vv tests/*_api.py
 

pyproject.toml

@@ -78,7 +78,7 @@ snuba = "snuba.cli:main"
 dev = [
     "devservices>=1.2.1",
     "freezegun>=1.5.5",
-    "mypy>=1.1.1",
+    "mypy>=1.18.2",
     "pre-commit>=4.2.0",
     "pytest>=9.0.3",
     "pytest-cov>=4.1.0",
@@ -96,31 +96,63 @@ dev = [
     "typing-extensions>=4.12.2",
 ]
 
+[tool.pytest.ini_options]
+python_files = "test*.py"
+addopts = "--tb=native -p no:doctest -p no:warnings"
+norecursedirs = "bin dist docs htmlcov script hooks node_modules .*"
+looponfailroots = ["snuba", "tests"]
+markers = [
+    "clickhouse_db: Use clickhouse",
+    "redis_db: Use redis",
+    "ci_only: Only run in CI",
+    "eap: Use clickhouse with EAP migrations only",
+    "genmetrics_db: Use clickhouse with generic metrics migrations only",
+]
+
 [tool.ruff]
 # File filtering is taken care of in pre-commit.
 line-length = 100
 target-version = "py313"
 
 [tool.ruff.lint]
 select = [
-    # todo: eventually we should get this enabled
-    # "B",     flake8-bugbear
-    "E", # pycodestyle errors
-    "F", # pyflakes
-    "W", # pycodestyle warnings
-    "I", # isort
+    "E",   # pycodestyle errors
+    "F",   # pyflakes
+    "W",   # pycodestyle warnings
+    "I",   # isort
+    "B",   # flake8-bugbear
+    "UP",  # pyupgrade
+    "C4",  # flake8-comprehensions
+    "SIM", # flake8-simplify
+    "RET", # flake8-return
 ]
 ignore = [
-    "E501", # line too long (handled by formatter)
-    "E402", # module level import not at top of file
+    "E501",   # line too long (handled by formatter)
+    "SIM108", # use ternary instead of if-else (often hurts readability)
+    "RET504", # unnecessary assignment before return (named results aid debugging)
+    # PEP 695 native generics/type-aliases are a deliberate migration, not a
+    # mechanical sweep — risky alongside custom metaclasses. Adopt case by case.
+    "UP046",  # non-pep695-generic-class
+    "UP047",  # non-pep695-generic-function
+    "UP040",  # non-pep695-type-alias
 ]
 
+[tool.ruff.lint.isort]
+known-first-party = ["snuba", "tests"]
+
 [tool.mypy]
 python_version = "3.13"
 strict = true
 ignore_missing_imports = false
 files = ["."]
-exclude = ["^rust_snuba/", "^tests/datasets/", "^tests/query/"]
+exclude = [
+    "^rust_snuba/",
+    "^tests/datasets/",
+    "^tests/query/",
+    # docker-only distributed-migration scaffolding; its top-level conftest.py
+    # collides with the repo-root conftest.py module name under `mypy .`
+    "^test_distributed_migrations/",
+]
 
 [[tool.mypy.overrides]]
 module = [

scripts/check-migrations.py

@@ -2,8 +2,8 @@
 import argparse
 import os
 import subprocess
+from collections.abc import Sequence
 from shutil import ExecError
-from typing import Optional, Sequence
 
 import requests
 
@@ -107,13 +107,10 @@ def _get_changes(globs: Sequence[str], workdir: str, to: str) -> str:
     )
     if changes.returncode != 0:
         raise ExecError(changes.stdout)
-    else:
-        return changes.stdout
+    return changes.stdout
 
 
-def main(
-    to: str = "origin/master", workdir: str = ".", labels: Optional[Sequence[str]] = []
-) -> None:
+def main(to: str = "origin/master", workdir: str = ".", labels: Sequence[str] | None = []) -> None:
     if labels:
         for label in labels:
             if SKIP_LABEL in label:

scripts/copy_tables.py

@@ -3,7 +3,7 @@
 import argparse
 import re
 from collections import OrderedDict
-from typing import Mapping, Optional, Sequence
+from collections.abc import Mapping, Sequence
 
 from clickhouse_driver import Client
 
@@ -100,7 +100,7 @@ def copy_tables(
     source_database: str,
     target_database: str,
     execute: bool,
-    tables: Optional[Sequence[str]],
+    tables: Sequence[str] | None,
 ) -> None:
     """
     When adding a replica to a clickhouse cluster, that node will not have any tables

scripts/ddl-changes.py

@@ -26,26 +26,25 @@ def _main() -> None:
     )
     if diff_result.returncode != 0:
         raise ExecError(diff_result.stdout)
-    else:
-        lines = diff_result.stdout.splitlines()
-        if len(lines) > 0:
-            print("-- start migrations")
-            print()
-        for line in lines:
-            migration_filename = os.path.basename(line)
-            migration_group = MigrationGroup(os.path.basename(os.path.dirname(line)))
-            migration_id, _ = os.path.splitext(migration_filename)
-            runner = Runner()
-            migration_key = MigrationKey(migration_group, migration_id)
-            print(f"-- forward migration {migration_group.value} : {migration_id}")
-            runner.run_migration(migration_key, dry_run=True)
-            print(f"-- end forward migration {migration_group.value} : {migration_id}")
+    lines = diff_result.stdout.splitlines()
+    if len(lines) > 0:
+        print("-- start migrations")
+        print()
+    for line in lines:
+        migration_filename = os.path.basename(line)
+        migration_group = MigrationGroup(os.path.basename(os.path.dirname(line)))
+        migration_id, _ = os.path.splitext(migration_filename)
+        runner = Runner()
+        migration_key = MigrationKey(migration_group, migration_id)
+        print(f"-- forward migration {migration_group.value} : {migration_id}")
+        runner.run_migration(migration_key, dry_run=True)
+        print(f"-- end forward migration {migration_group.value} : {migration_id}")
 
-            print("\n\n\n")
-            migration_key = MigrationKey(migration_group, migration_id)
-            print(f"-- backward migration {migration_group.value} : {migration_id}")
-            runner.reverse_migration(migration_key, dry_run=True)
-            print(f"-- end backward migration {migration_group.value} : {migration_id}")
+        print("\n\n\n")
+        migration_key = MigrationKey(migration_group, migration_id)
+        print(f"-- backward migration {migration_group.value} : {migration_id}")
+        runner.reverse_migration(migration_key, dry_run=True)
+        print(f"-- end backward migration {migration_group.value} : {migration_id}")
 
 
 if __name__ == "__main__":

scripts/fetch_service_refs.py

@@ -8,20 +8,20 @@
 import time
 import urllib.error
 import urllib.request
-from typing import Any, Dict, Optional
+from typing import Any
 
 GO_SERVER_URL = os.environ["GO_SERVER_URL"]
 
 # the maximum number of times to fetch the pipeline history
 MAX_FETCHES = 100
 
 
-def pipeline_passed(pipeline: Dict[str, Any]) -> bool:
-    stage_status_dict: Dict[str, str] = {
+def pipeline_passed(pipeline: dict[str, Any]) -> bool:
+    stage_status_dict: dict[str, str] = {
         stage["name"]: stage["status"] for stage in pipeline["stages"]
     }
 
-    return stage_status_dict.get("pipeline-complete", None) == "Passed"
+    return stage_status_dict.get("pipeline-complete") == "Passed"
 
 
 # print the most recent passing sha for a repo
@@ -33,7 +33,7 @@ def main(pipeline_name: str = "deploy-snuba-us", repo: str = "snuba") -> int:
             GOCD_ACCESS_TOKEN not set. It should be an access token belonging to bot@sentry.io.
             """
         )
-    fetch_url: Optional[str] = f"{GO_SERVER_URL}/api/pipelines/{pipeline_name}/history"
+    fetch_url: str | None = f"{GO_SERVER_URL}/api/pipelines/{pipeline_name}/history"
     fetches = 0
     while fetch_url and fetches < MAX_FETCHES:
         fetches += 1
@@ -47,7 +47,7 @@ def main(pipeline_name: str = "deploy-snuba-us", repo: str = "snuba") -> int:
         try:
             resp = urllib.request.urlopen(req)
         except urllib.error.HTTPError as e:
-            raise SystemExit(f"Failed to fetch pipeline history:\n{e.read().decode()}")
+            raise SystemExit(f"Failed to fetch pipeline history:\n{e.read().decode()}") from e
 
         print("fetching pipeline history for", pipeline_name, fetch_url, file=sys.stderr)
         data = json.loads(resp.read())
@@ -65,11 +65,11 @@ def main(pipeline_name: str = "deploy-snuba-us", repo: str = "snuba") -> int:
                 for r in pipeline["build_cause"]["material_revisions"]:
                     # example material description format... `in` is good enough
                     # 'URL: git@github.com:getsentry/devinfra-example-service.git, Branch: main'
-                    if f"git@github.com:getsentry/{repo}.git" in r["material"]["description"]:
-                        rev = r["modifications"][0]["revision"]
-                        print(rev)
-                        return 0
-                    elif f"https://github.com/getsentry/{repo}.git" in r["material"]["description"]:
+                    if (
+                        f"git@github.com:getsentry/{repo}.git" in r["material"]["description"]
+                        or f"https://github.com/getsentry/{repo}.git"
+                        in r["material"]["description"]
+                    ):
                         rev = r["modifications"][0]["revision"]
                         print(rev)
                         return 0

scripts/generate_items.py

@@ -1,10 +1,9 @@
 import time
 import uuid
 from datetime import UTC, datetime, timedelta
-from typing import Optional
 
+from confluent_kafka import KafkaError, Producer
 from confluent_kafka import Message as KafkaMessage
-from confluent_kafka import Producer
 from google.protobuf.timestamp_pb2 import Timestamp
 from sentry_protos.snuba.v1.request_common_pb2 import TraceItemType
 from sentry_protos.snuba.v1.trace_item_pb2 import AnyValue, TraceItem
@@ -15,12 +14,12 @@
 producer = Producer(kafka_config)
 
 
-def delivery_report(err: Optional[Exception], msg: KafkaMessage) -> None:
+def delivery_report(err: KafkaError | None, msg: KafkaMessage) -> None:
     if err is not None:
         print(f"Message delivery failed: {err}")
 
 
-def generate_item_message(start_timestamp: Optional[datetime] = None) -> bytes:
+def generate_item_message(start_timestamp: datetime | None = None) -> bytes:
     if start_timestamp is None:
         start_timestamp = datetime.now(tz=UTC)
 

snuba/admin/audit_log/base.py

@@ -1,5 +1,6 @@
-from datetime import datetime, timezone
-from typing import Any, Mapping, MutableMapping, Optional, Union
+from collections.abc import Mapping, MutableMapping
+from datetime import UTC, datetime
+from typing import Any
 
 import structlog
 
@@ -24,10 +25,10 @@ def record(
         self,
         user: str,
         action: AuditLogAction,
-        data: Mapping[str, Union[str, int]],
-        notify: Optional[bool] = False,
+        data: Mapping[str, str | int],
+        notify: bool | None = False,
     ) -> None:
-        timestamp = datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%S.%fZ")
+        timestamp = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S.%fZ")
         self.logger.info(
             event=action.value,
             user=user,

snuba/admin/audit_log/query.py

@@ -1,15 +1,16 @@
 from __future__ import annotations
 
-from datetime import datetime, timezone
+from collections.abc import Callable, MutableMapping
+from datetime import UTC, datetime
 from enum import Enum
 from functools import partial
-from typing import Callable, MutableMapping, TypeVar, Union
-
-DATETIME_FORMAT = "%Y-%m-%dT%H:%M:%S.%fZ"
+from typing import TypeVar
 
 from snuba.admin.audit_log.action import AuditLogAction
 from snuba.admin.audit_log.base import AuditLog
 
+DATETIME_FORMAT = "%Y-%m-%dT%H:%M:%S.%fZ"
+
 Return = TypeVar("Return")
 
 
@@ -30,7 +31,7 @@ def audit_log(fn: Callable[[str, str], Return]) -> Callable[[str, str], Return]:
     """
 
     def audit_log_wrapper(query: str, user: str) -> Return:
-        data: MutableMapping[str, Union[str, QueryExecutionStatus]] = {
+        data: MutableMapping[str, str | int] = {
             "query": query,
         }
         audit_log_notify = partial(
@@ -42,11 +43,11 @@ def audit_log_wrapper(query: str, user: str) -> Return:
             result = fn(query, user)
         except Exception:
             data["status"] = QueryExecutionStatus.FAILED.value
-            data["end_timestamp"] = datetime.now(timezone.utc).strftime(DATETIME_FORMAT)
+            data["end_timestamp"] = datetime.now(UTC).strftime(DATETIME_FORMAT)
             audit_log_notify(data=data)
             raise
         data["status"] = QueryExecutionStatus.SUCCEEDED.value
-        data["end_timestamp"] = datetime.now(timezone.utc).strftime(DATETIME_FORMAT)
+        data["end_timestamp"] = datetime.now(UTC).strftime(DATETIME_FORMAT)
         audit_log_notify(data=data)
         return result
 

snuba/admin/auth.py

@@ -1,7 +1,7 @@
 from __future__ import annotations
 
 import json
-from typing import Sequence
+from collections.abc import Sequence
 
 import rapidjson
 import structlog
@@ -48,7 +48,7 @@ def _is_member_of_group(user: AdminUser, group: str) -> bool:
 def get_iam_roles_from_user(user: AdminUser) -> Sequence[str]:
     iam_roles = []
     try:
-        with open(settings.ADMIN_IAM_POLICY_FILE, "r") as policy_file:
+        with open(settings.ADMIN_IAM_POLICY_FILE) as policy_file:
             policy = json.load(policy_file)
             for binding in policy["bindings"]:
                 role: str = binding["role"].split("roles/")[-1]