Skip to content

Commit

Permalink
Move to using the sqlalchemy mypy plugin
Browse files Browse the repository at this point in the history
This provides better type checking against the sqlalchemy code
and allows us to remove our stubs and the 'type: ignore's which
were in various places where the stubs didn't match reality.
  • Loading branch information
PeterJCLaw committed Sep 25, 2022
1 parent 47d99d2 commit 8a5f8f9
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 85 deletions.
5 changes: 2 additions & 3 deletions routemaster/conftest.py
Expand Up @@ -326,7 +326,7 @@ def database_clear(app: TestApp) -> Iterator[None]:
with app.new_session():
for table in metadata.tables:
app.session.execute(
f'truncate table {table} cascade',
f'truncate table {table} cascade', # type: ignore[arg-type] # noqa: E501
{},
)

Expand Down Expand Up @@ -514,8 +514,7 @@ def _inner(label: LabelRef) -> str:
label_name=label.name,
label_state_machine=label.state_machine,
).order_by(
# TODO: use the sqlalchemy mypy plugin rather than our stubs
History.id.desc(), # type: ignore[attr-defined]
History.id.desc(),
).limit(1).scalar()
return _inner

Expand Down
29 changes: 24 additions & 5 deletions routemaster/db/model.py
@@ -1,7 +1,7 @@
"""Database model definition."""
import datetime
import functools
from typing import Any
from typing import Any, Dict, List, Optional

import dateutil.tz
from sqlalchemy import DDL, Table
Expand All @@ -16,7 +16,7 @@
ForeignKeyConstraint,
func,
)
from sqlalchemy.orm import relationship
from sqlalchemy.orm import Mapped, relationship
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.dialects.postgresql import JSONB

Expand Down Expand Up @@ -53,7 +53,8 @@
class Label(Base):
"""A single label including context."""

# Note: type annotations for this class are provided by a stubs file
# Note: type annotations provided below must be manually kept in sync with
# the fields defined in the Table.

__table__ = Table(
'labels',
Expand All @@ -74,6 +75,15 @@ class Label(Base):
],
)

name: Mapped[str]
state_machine: Mapped[str]
metadata: Mapped[Dict[str, Any]]
metadata_triggers_processed: Mapped[bool]
deleted: Mapped[bool]
updated: Mapped[datetime.datetime]

history: List['History'] = relationship('History')

def __repr__(self) -> str:
"""Return a useful debug representation."""
return (
Expand All @@ -84,7 +94,8 @@ def __repr__(self) -> str:
class History(Base):
"""A single historical state transition of a label."""

# Note: type annotations for this class are provided by a stubs file
# Note: type annotations provided below must be manually kept in sync with
# the fields defined in the Table.

__table__ = Table(
'history',
Expand Down Expand Up @@ -115,7 +126,15 @@ class History(Base):
NullableColumn('new_state', String),
)

label = relationship(Label, backref='history')
id: Mapped[int]
label_name: Mapped[str]
label_state_machine: Mapped[str]
created: Mapped[datetime.datetime]
forced: Mapped[bool]
old_state: Mapped[Optional[str]]
new_state: Mapped[Optional[str]]

label: Label = relationship(Label)

def __repr__(self) -> str:
"""Return a useful debug representation."""
Expand Down
60 changes: 0 additions & 60 deletions routemaster/db/model.pyi

This file was deleted.

2 changes: 1 addition & 1 deletion routemaster/state_machine/api.py
Expand Up @@ -108,7 +108,7 @@ def update_metadata_for_label(
)

# FIXME: handle cases where metadata aren't dicts.
new_metadata = dict_merge(existing_metadata, update) # type: ignore[arg-type] # noqa: E501
new_metadata = dict_merge(existing_metadata, update)

row.metadata = new_metadata
row.metadata_triggers_processed = not needs_gate_evaluation
Expand Down
17 changes: 4 additions & 13 deletions routemaster/state_machine/utils.py
Expand Up @@ -101,11 +101,7 @@ def get_current_history(app: App, label: LabelRef) -> History:
label_name=label.name,
label_state_machine=label.state_machine,
).order_by(
# Our model type stubs define the `id` attribute as `int`, yet
# sqlalchemy actually allows the attribute to be used for ordering like
# this; ignore the type check here specifically rather than complicate
# our type definitions.
History.id.desc(), # type: ignore[attr-defined]
History.id.desc(),
).first()

if history_entry is None:
Expand Down Expand Up @@ -185,14 +181,13 @@ def labels_in_state_with_metadata(

metadata_lookup = Label.metadata
for part in path:
metadata_lookup = metadata_lookup[part] # type: ignore[call-overload, index] # noqa: E501
metadata_lookup = metadata_lookup[part] # type: ignore[assignment] # noqa: E501

return _labels_in_state(
app,
state_machine,
state,
# TODO: use the sqlalchemy mypy plugin rather than our stubs file
metadata_lookup.astext.in_(values), # type: ignore[union-attr]
metadata_lookup.astext.in_(values),
)


Expand Down Expand Up @@ -228,11 +223,7 @@ def _labels_in_state(
History.label_name,
History.new_state,
func.row_number().over(
# Our model type stubs define the `id` attribute as `int`, yet
# sqlalchemy actually allows the attribute to be used for ordering
# like this; ignore the type check here specifically rather than
# complicate our type definitions.
order_by=History.id.desc(), # type: ignore[attr-defined]
order_by=History.id.desc(),
partition_by=History.label_name,
).label('rank'),
).filter_by(
Expand Down
3 changes: 1 addition & 2 deletions routemaster/validation.py
Expand Up @@ -78,8 +78,7 @@ def _validate_no_labels_in_nonexistent_states(
History.label_name,
History.new_state,
func.row_number().over(
# TODO: use the sqlalchemy mypy plugin rather than our stubs file
order_by=History.id.desc(), # type: ignore[attr-defined]
order_by=History.id.desc(),
partition_by=History.label_name,
).label('rank'),
).filter_by(
Expand Down
1 change: 1 addition & 0 deletions setup.cfg
Expand Up @@ -18,6 +18,7 @@ strict_optional=true
show_error_codes = true
enable_error_code = ignore-without-code
warn_unused_ignores = true
plugins = sqlalchemy.ext.mypy.plugin

[coverage:run]
branch=True
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Expand Up @@ -50,7 +50,7 @@
'jsonschema >=3, <5',
'flask',
'psycopg2-binary',
'sqlalchemy',
'sqlalchemy[mypy]',
'python-dateutil',
'alembic >=0.9.6',
'gunicorn >=19.7',
Expand Down

0 comments on commit 8a5f8f9

Please sign in to comment.