mirror of
https://github.com/kuhyx/WUT_Computer_Science.git
synced 2026-11-10 17:47:14 +01:00
The six mypy errors are gone: four were fixable, two are scoped
The note in TODO-refactor-remaining.md said ERSMS's four were structural and that a fix "would rewrite both models". Both halves were wrong. flask-sqlalchemy 3.1.1 does ship py.typed; the real blocker was that `class User(db.Model)` is attribute access, which mypy cannot resolve in a base-class position. backend/models.py now declares a named DeclarativeBase with SQLAlchemy 2.0 Mapped[] columns. The Model mixin goes away with the base, so the six legacy Model.query call sites became db.session selects and the two table names are spelled out rather than derived. Verified by driving the real routes through Flask's test client with Firebase stubbed out -- 16 scenarios covering every one of the six call sites, plus both CREATE TABLE statements, byte-identical to the submitted version. That harness caught the one thing review would not have: mapped_column(ForeignKey( "user.uid")) silently infers VARCHAR(80) from the referenced column, which had changed the submitted `user_id INTEGER`. An explicit Integer puts it back. app.py went over the 250-line cap on the way, hence the split; models.py takes the shebang + exec bit that NLP's nine import-only modules already use, since __init__.py would make `from models import` unresolvable to mypy while it still works at runtime. backend/Dockerfile copied only app.py, so it needed the new file too. PSD's two are pyflink's MapFunction and KeyedProcessFunction, which ship no types and are not installed in that venv at all -- a hand-written .pyi would assert an API nothing here can check. A [[tool.mypy.overrides]] scoped to that one module turns off disallow_subclassing_any and nothing else. ERSMS 4 -> 0, PSD 2 -> 0, under --strict. ruff and ruff format clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0172dj34FDpao6wsix8ZuCzE
This commit is contained in:
parent
37cc00fa21
commit
2a59de6363
@ -5,8 +5,12 @@
|
||||
# already clean, so `pre-commit run --all-files` is green at every commit.
|
||||
# Hooks still to come, each blocked on its own cleanup:
|
||||
# - file-length blocked on the .tex/code splits and TODO-file-length-250.md
|
||||
# - mypy blocked on six errors that are third-party typing
|
||||
# limits, not our code -- see TODO-refactor-remaining.md
|
||||
# - mypy the six third-party-typing errors are GONE (ERSMS moved
|
||||
# to a typed DeclarativeBase; PSD has a scoped override).
|
||||
# Still blocked on ECOTE/NLP/EARIN, and on the fact that
|
||||
# mypy cannot run repo-wide -- many courses ship a main.py
|
||||
# and it stops at "Duplicate module named". It needs a
|
||||
# per-directory driver, not just a clean tree.
|
||||
# - dependency-freshness blocked on the upgrade itself; the gate and the
|
||||
# upgrade must land in the SAME commit or the repo is
|
||||
# instantly and permanently red
|
||||
|
||||
@ -11,6 +11,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
|
||||
|
||||
# Copy the Python script, requirements, and constants.ini file into the container at /app
|
||||
COPY ./app.py /app/
|
||||
COPY ./models.py /app/
|
||||
|
||||
COPY requirements.txt .
|
||||
COPY .env .
|
||||
|
||||
@ -19,6 +19,7 @@ from firebase_admin import auth, credentials, exceptions
|
||||
from flask import Flask, jsonify, request
|
||||
from flask_cors import CORS
|
||||
from flask_sqlalchemy import SQLAlchemy
|
||||
from models import Base, Rating, User
|
||||
from sqlalchemy.exc import SQLAlchemyError
|
||||
|
||||
if TYPE_CHECKING:
|
||||
@ -44,7 +45,7 @@ app = Flask(__name__)
|
||||
app.config["SQLALCHEMY_DATABASE_URI"] = "sqlite:///app.sqlite3"
|
||||
app.config["SQLALCHEY_TRACK_MODIFICATIONS"] = False
|
||||
|
||||
db = SQLAlchemy(app)
|
||||
db = SQLAlchemy(app, model_class=Base)
|
||||
|
||||
CORS(app)
|
||||
|
||||
@ -52,24 +53,6 @@ cred = credentials.Certificate("movie-recommendation-firebase-adminsdk.json")
|
||||
firebase_admin.initialize_app(cred)
|
||||
|
||||
|
||||
class User(db.Model):
|
||||
"""One Firebase account, remembered so ratings have an owner."""
|
||||
|
||||
id = db.Column(db.Integer, primary_key=True)
|
||||
uid = db.Column(db.String(80), unique=True, nullable=False)
|
||||
email = db.Column(db.String(80), unique=True, nullable=False)
|
||||
is_admin = db.Column(db.Boolean, default=False)
|
||||
|
||||
|
||||
class Rating(db.Model):
|
||||
"""One user's score for one film."""
|
||||
|
||||
id = db.Column(db.Integer, primary_key=True)
|
||||
user_id = db.Column(db.Integer, db.ForeignKey("user.uid"), nullable=False)
|
||||
movie_id = db.Column(db.Integer, nullable=False)
|
||||
value = db.Column(db.Integer, nullable=False)
|
||||
|
||||
|
||||
with app.app_context():
|
||||
db.create_all()
|
||||
|
||||
@ -84,7 +67,7 @@ def login() -> ResponseReturnValue:
|
||||
logger.info("login: %s", uid)
|
||||
email = decoded_token.get("email")
|
||||
|
||||
user = User.query.filter_by(uid=uid).first()
|
||||
user = db.session.scalars(db.select(User).filter_by(uid=uid)).first()
|
||||
if user is None:
|
||||
user = User(uid=uid, email=email)
|
||||
|
||||
@ -106,11 +89,13 @@ def count_user_ratings() -> ResponseReturnValue:
|
||||
decoded_token = auth.verify_id_token(token)
|
||||
user_id = decoded_token["uid"]
|
||||
|
||||
user = User.query.filter_by(uid=user_id).first()
|
||||
user = db.session.scalars(db.select(User).filter_by(uid=user_id)).first()
|
||||
if user is None:
|
||||
return jsonify({"message": "Error"}), HTTP_INTERNAL_SERVER_ERROR
|
||||
|
||||
rating_count = Rating.query.filter_by(user_id=user_id).count()
|
||||
rating_count = db.session.scalar(
|
||||
db.select(db.func.count()).select_from(Rating).filter_by(user_id=user_id)
|
||||
)
|
||||
except (*AUTH_ERRORS, SQLAlchemyError) as exc:
|
||||
logger.exception("Could not count ratings")
|
||||
return jsonify({"message": str(exc)}), HTTP_INTERNAL_SERVER_ERROR
|
||||
@ -166,7 +151,9 @@ def add_rating() -> ResponseReturnValue:
|
||||
decoded_token = auth.verify_id_token(token)
|
||||
user_id = decoded_token["uid"]
|
||||
|
||||
rating = Rating.query.filter_by(user_id=user_id, movie_id=movie).first()
|
||||
rating = db.session.scalars(
|
||||
db.select(Rating).filter_by(user_id=user_id, movie_id=movie)
|
||||
).first()
|
||||
if rating is None:
|
||||
rating = Rating(user_id=user_id, movie_id=movie, value=value)
|
||||
|
||||
@ -192,7 +179,9 @@ def remove_rating() -> ResponseReturnValue:
|
||||
decoded_token = auth.verify_id_token(token)
|
||||
user_id = decoded_token["uid"]
|
||||
|
||||
rating = Rating.query.filter_by(user_id=user_id, movie_id=movie).first()
|
||||
rating = db.session.scalars(
|
||||
db.select(Rating).filter_by(user_id=user_id, movie_id=movie)
|
||||
).first()
|
||||
if rating is None:
|
||||
return jsonify({"message": "Error"}), HTTP_INTERNAL_SERVER_ERROR
|
||||
|
||||
@ -213,7 +202,9 @@ def get_rating() -> ResponseReturnValue:
|
||||
decoded_token = auth.verify_id_token(token)
|
||||
user_id = decoded_token["uid"]
|
||||
|
||||
rating = Rating.query.filter_by(user_id=user_id, movie_id=movie).first()
|
||||
rating = db.session.scalars(
|
||||
db.select(Rating).filter_by(user_id=user_id, movie_id=movie)
|
||||
).first()
|
||||
if rating is None:
|
||||
return jsonify({"message": "Rating not found!"}), HTTP_OK
|
||||
except (*AUTH_ERRORS, SQLAlchemyError) as exc:
|
||||
|
||||
52
Programming/ERSMS-project/backend/models.py
Executable file
52
Programming/ERSMS-project/backend/models.py
Executable file
@ -0,0 +1,52 @@
|
||||
#!/usr/bin/env python3
|
||||
"""The two persisted tables: an account, and one account's score for one film.
|
||||
|
||||
Split out of `app.py`, which went over this repo's 250-line cap when the models
|
||||
were migrated to SQLAlchemy 2.0's typed declarative style. They import nothing
|
||||
from `app`, so there is no cycle: `app.py` builds the `SQLAlchemy` object from
|
||||
`Base` rather than the models being built from it.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from sqlalchemy import ForeignKey, Integer, String
|
||||
from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column
|
||||
|
||||
|
||||
class Base(DeclarativeBase):
|
||||
"""Declarative base for this app's two models.
|
||||
|
||||
`class User(db.Model)` was attribute access, which mypy cannot resolve in a
|
||||
base-class position -- that was four `--strict` errors. A named base fixes
|
||||
it. The trade is that flask-sqlalchemy's `Model` mixin goes with it, so
|
||||
table names are spelled out here rather than derived from the class name.
|
||||
"""
|
||||
|
||||
|
||||
class User(Base):
|
||||
"""One Firebase account, remembered so ratings have an owner."""
|
||||
|
||||
__tablename__ = "user"
|
||||
|
||||
id: Mapped[int] = mapped_column(primary_key=True)
|
||||
uid: Mapped[str] = mapped_column(String(80), unique=True)
|
||||
email: Mapped[str] = mapped_column(String(80), unique=True)
|
||||
# Nullable with a default, exactly as the submitted schema had it.
|
||||
is_admin: Mapped[bool | None] = mapped_column(default=False)
|
||||
|
||||
|
||||
class Rating(Base):
|
||||
"""One user's score for one film."""
|
||||
|
||||
__tablename__ = "rating"
|
||||
|
||||
id: Mapped[int] = mapped_column(primary_key=True)
|
||||
# As handed in: the foreign key points at user.uid (a VARCHAR) while the
|
||||
# column itself is declared INTEGER, and what actually gets stored is the
|
||||
# Firebase uid string -- SQLite is dynamically typed, so this worked. The
|
||||
# explicit Integer keeps the submitted DDL byte-identical, because
|
||||
# mapped_column otherwise infers VARCHAR(80) from the referenced column;
|
||||
# the `str` annotation is what the code really puts in it.
|
||||
user_id: Mapped[str] = mapped_column(Integer, ForeignKey("user.uid"))
|
||||
movie_id: Mapped[int]
|
||||
value: Mapped[int]
|
||||
@ -94,6 +94,16 @@ module = [
|
||||
]
|
||||
ignore_missing_imports = true
|
||||
|
||||
# pyflink ships no types AND is not installed in Programming/PSD/.venv, so
|
||||
# `MapFunction` and `KeyedProcessFunction` are `Any` and --strict (which
|
||||
# implies disallow_subclassing_any) refuses to subclass them. Scoped to the one
|
||||
# module that has to subclass them; everything else in PSD is clean under
|
||||
# --strict. A hand-written .pyi was the alternative and is worse: it would
|
||||
# assert an API that nothing here can check against.
|
||||
[[tool.mypy.overrides]]
|
||||
module = ["temperature_anomaly_detector"]
|
||||
disallow_subclassing_any = false
|
||||
|
||||
[tool.coverage.run]
|
||||
branch = true
|
||||
omit = [
|
||||
|
||||
Loading…
Reference in New Issue
Block a user