Skip to content

Commit 570d4d2

Browse files
authored
Merge branch 'main' into copilot/fix-browserslist-vulnerability
2 parents 6939c0d + d35925f commit 570d4d2

29 files changed

Lines changed: 103 additions & 756 deletions

.github/instructions/python-docstrings.instructions.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ The reverse of overstating: when a docstring uses applicability language (`Use w
8383
- **Describe intent, not a code-controlled enumeration** — a docstring listing the members of a set the code controls (a `parametrize` list, a module constant, registry entries) goes stale the moment the set changes. Name the intent and the controlling symbol ("every app in `BESPOKE_BASE_APP_PLUGINS`"), not the members. Applies in `tests/` too.
8484
- **Framework/core symbols stay app-agnostic** — a shared framework/core symbol's docstring describes its behaviour in generic terms; naming one downstream app's domain concept (`backup_type`, `snippet_filename`) couples the abstraction's contract to one consumer. Restate as the generic role.
8585
- **State the role; don't pin what the reader can grep.** Two shapes rot on the next change and are never edited at the change site: **a count** ("read by seven non-alerts apps", "the only two callers") and **an enumeration of callers or consumers**. Write the *property* instead — not "read by seven non-alerts apps" but "read by every app offering `alert_on_fail`"; not "used by `a.py`, `b.py`, `c.py`" but "shared across the subtree". When the enumeration is genuinely load-bearing, the enforcement belongs in code (a registry, an `__all__`, a guard), with the docstring pointing at it — prose is not a mechanism.
86-
- **A parity claim is scoped to the members it covers.** "The expected row orders are the same literals `TestListQueryPaginatedPostgres` asserts" reads as a guarantee over the whole class; when it holds for only some members it is false for the rest and nothing marks which. Scope the assertion and name the exceptions ("…the all-NULL and `select_related` cases are MySQL-only"). Cross-dialect test classes are the recurring shape.
86+
- **A parity claim is scoped to the members it covers.** "The expected row orders are the same literals `TestListQueryPaginatedPostgres` asserts" reads as a guarantee over the whole class; when it holds for only some members it is false for the rest and nothing marks which. Scope the assertion and name the exceptions ("…the all-NULL case is PostgreSQL-only"). Cross-dialect test classes are the recurring shape.
8787

8888
## Say it once, in the surface that owns it
8989

.github/instructions/python-duplication.instructions.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ Concrete cases: `3306`/`5432` ports → `DEFAULT_MYSQL_PORT`/`DEFAULT_POSTGRESQL
4242
| `len(v) > 0` `field_validator` | `NonEmptyStr` |
4343
| `.strip().lower()` `field_validator` | `Annotated[str, StringConstraints(strip_whitespace=True, to_lower=True)]` or `LowercaseStr` |
4444
| `field_validator` doing a string-*shape* check (split on a separator, reject empty halves, reject stray whitespace) | `Annotated[str, StringConstraints(pattern=...)]` field type |
45-
| `.nulls_last()` on an `ORDER BY` term | `app/core/db/utils.py::NullsLastOrdering(column, *, descending=False)``.nulls_last()` emits SQL MySQL cannot parse. Pass the bare column plus `descending=`, never a pre-`desc()`-ed expression |
45+
| `.nulls_last()` on an `ORDER BY` term | `app/core/db/utils.py::NullsLastOrdering(column, *, descending=False)`one shared cache-keyed construct; pass the bare column plus `descending=`, never a pre-`desc()`-ed expression (which would render `<expr> DESC ASC NULLS LAST`) |
4646

4747
**Rule of thumb.** If a new decorator or class has 15+ lines of state management (timestamps, eviction, key hashing, TTL math), ask "why isn't this `@alru_cache` or `@ttl_cache`?" Flag as **Important**.
4848

.github/workflows/python.yaml

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ jobs:
8080
- name: Install test dependencies
8181
run: poetry sync --no-root
8282
- name: Run tests
83-
run: make test PYTEST_MARKERS='not postgres and not mysql' COV=${{ matrix.python == '3.11.9' && '1' || '0' }}
83+
run: make test PYTEST_MARKERS='not postgres' COV=${{ matrix.python == '3.11.9' && '1' || '0' }}
8484
- name: Generate coverage comment data
8585
id: coverage_comment
8686
if: matrix.python == '3.11.9' && github.event_name == 'pull_request'
@@ -125,34 +125,6 @@ jobs:
125125
- name: Run PostgreSQL tests
126126
run: make test PYTEST_MARKERS='postgres' PYTEST_WORKERS=0 COV=0
127127

128-
test_mysql:
129-
name: test (mysql)
130-
runs-on: ubuntu-latest
131-
services:
132-
mysql:
133-
image: mysql:8.0.46
134-
env:
135-
MYSQL_ROOT_PASSWORD: sep
136-
MYSQL_DATABASE: sep_test
137-
ports: ["3306:3306"]
138-
options: >-
139-
--health-cmd "mysqladmin ping -h 127.0.0.1 -psep --silent"
140-
--health-interval 10s --health-timeout 5s --health-retries 10
141-
env:
142-
SEP_TEST_MYSQL_DSN: mysql+aiomysql://root:sep@127.0.0.1:3306/sep_test
143-
steps:
144-
- name: Checkout codebase
145-
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
146-
- name: Setup Python
147-
uses: ./.github/actions/setup-python-job
148-
with:
149-
python-version: "3.11.9"
150-
cache: poetry
151-
- name: Install test dependencies
152-
run: poetry sync --no-root --with mysql
153-
- name: Run MySQL tests
154-
run: make test PYTEST_MARKERS='mysql' PYTEST_WORKERS=0 COV=0
155-
156128
build:
157129
strategy:
158130
matrix:

README.md

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ SEP supports multiple database engines for different components. Each component
403403
```yaml
404404
SEP:
405405
DATABASE:
406-
ENGINE: sqlite # Database engine: sqlite, mysql, postgresql
406+
ENGINE: sqlite # Database engine: sqlite, postgresql
407407
USER: null
408408
PASSWORD: null
409409
HOST: "" # Database host (empty string for SQLite to avoid URL construction issues)
@@ -429,18 +429,6 @@ TASKS:
429429
NAME: tasks.db
430430
```
431431

432-
#### MySQL/MariaDB Configuration
433-
```yaml
434-
SEP:
435-
DATABASE:
436-
ENGINE: mysql
437-
USER: sep_user
438-
PASSWORD: your_secure_password
439-
HOST: localhost
440-
PORT: 3306
441-
NAME: sep_database
442-
```
443-
444432
#### PostgreSQL Configuration
445433
```yaml
446434
SEP:
@@ -455,7 +443,6 @@ SEP:
455443

456444
Supported database engines:
457445
- `sqlite`: SQLite database (default for development)
458-
- `mysql`: MySQL/MariaDB database
459446
- `postgresql`: PostgreSQL database
460447

461448
> [!NOTE]

app/core/db/config.py

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,18 +30,17 @@
3030
from app.core.utils.fields import AsyncDatabaseEngine
3131

3232
#: The driver kwarg each async dialect uses for its connect timeout. asyncpg
33-
#: takes ``timeout``, aiomysql takes ``connect_timeout``; aiosqlite's
34-
#: ``timeout`` means lock wait, not connect, so SQLite is absent.
33+
#: takes ``timeout``; aiosqlite's ``timeout`` means lock wait, not connect, so
34+
#: SQLite is absent.
3535
_CONNECT_TIMEOUT_KEYS: dict[AsyncDatabaseEngine, str] = {
3636
AsyncDatabaseEngine.POSTGRESQL: "timeout",
37-
AsyncDatabaseEngine.MYSQL: "connect_timeout",
3837
}
3938

4039

4140
class DatabaseOptions(BaseModel):
4241
"""Define configuration options for a database connection.
4342
44-
:param ENGINE: The database engine to use (e.g., SQLite, MySQL, PostgreSQL).
43+
:param ENGINE: The database engine to use (e.g., SQLite, PostgreSQL).
4544
Defaults to SQLite.
4645
:param USER: The username for the database connection.
4746
:param PASSWORD: The password for the database connection.
@@ -58,9 +57,8 @@ class DatabaseOptions(BaseModel):
5857
SQLAlchemy's default. Must be ``> 0``.
5958
:param CONNECT_TIMEOUT: Seconds to wait for a TCP connect. Unset passes no
6059
``connect_args``, leaving the driver's own default. Forwarded as
61-
``timeout`` for asyncpg and ``connect_timeout`` for aiomysql; omitted
62-
for SQLite, where that key means lock wait rather than connect. Must
63-
be ``> 0``.
60+
``timeout`` for asyncpg; omitted for SQLite, where that key means lock
61+
wait rather than connect. Must be ``> 0``.
6462
:param POOL_PRE_PING: Whether to test each pooled connection for liveness
6563
before handing it out. Defaults to ``True`` so a dead connection is
6664
discarded and replaced transparently. Unlike the sizing fields, this

app/core/db/crud.py

Lines changed: 16 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@
5151
PaginatedResponse,
5252
Pagination,
5353
)
54-
from app.core.utils.fields import DatabaseDialect
5554

5655
logger = logging.getLogger(__name__)
5756

@@ -296,40 +295,6 @@ async def _mutate_where(
296295
await session.commit()
297296
return result
298297

299-
@classmethod
300-
async def _mutate_where_returning_with_for_update(
301-
cls,
302-
session: AsyncSession,
303-
builder: _QueryBuilder,
304-
*whereclause: ColumnExpressionArgument[bool],
305-
returning: Iterable[str] | bool,
306-
**equal_filters: Any,
307-
) -> list[Any]:
308-
"""Execute a DML statement with `FOR UPDATE`.
309-
310-
This method is a workaround for MySQL, which does not support `RETURNING` in
311-
UPDATE/DELETE statements. It first selects the rows with `FOR UPDATE`, then
312-
executes the DML statement, and finally returns the selected rows.
313-
"""
314-
query = cls._build_query(
315-
*whereclause, builder=_select_builder(col(cls.Model.id)), **equal_filters
316-
).with_for_update()
317-
result = await cls._exec(session, query)
318-
319-
if row_ids := result.all():
320-
ids_filter = col(cls.Model.id).in_(row_ids)
321-
await cls._mutate_where(session, builder, ids_filter, returning=False)
322-
else:
323-
return []
324-
325-
if returning is True:
326-
return await cls.list(session, ids_filter)
327-
328-
if set(returning) == {"id"}:
329-
return row_ids
330-
331-
return await cls.values_list(session, returning, ids_filter)
332-
333298
@classmethod
334299
async def _dml_where(
335300
cls,
@@ -342,23 +307,24 @@ async def _dml_where(
342307
"""Execute a DML statement (UPDATE or DELETE) with the specified filters.
343308
344309
This method ensures that at least one filter is provided to avoid unintentional
345-
mass updates or deletions, and checks for database dialect-specific handling of
346-
the `RETURNING` clause.
310+
mass updates or deletions.
311+
312+
:param session: The SQLAlchemy asynchronous session to use for database
313+
operations.
314+
:param builder: The builder producing the UPDATE or DELETE statement.
315+
:param whereclause: The filter expressions applied to the statement.
316+
:param returning: The column names to return, ``True`` for whole rows, or
317+
``False`` for none.
318+
:param equal_filters: Additional equality filters applied to the statement.
319+
:return: The raw cursor result when nothing is returned, a list of rows when
320+
more than one column is requested, or a list of scalars for a single one.
321+
:raises ValueError: If neither ``whereclause`` nor ``equal_filters`` is given.
347322
"""
348323
if not whereclause and not equal_filters:
349324
raise ValueError(
350325
"You must specify at least one filter in *whereclause or **equal_filters"
351326
)
352327

353-
if returning and session.get_bind().name == DatabaseDialect.MYSQL:
354-
return await cls._mutate_where_returning_with_for_update(
355-
session,
356-
builder,
357-
*whereclause,
358-
returning=returning,
359-
**equal_filters,
360-
)
361-
362328
result = await cls._mutate_where(
363329
session,
364330
builder,
@@ -841,26 +807,21 @@ async def get_or_create(
841807
instance exists, it returns it. Otherwise, it creates and saves a new one.
842808
843809
The creation step is conflict-tolerant: it uses a dialect-aware idempotent
844-
insert (``INSERT ... ON CONFLICT DO NOTHING`` / ``INSERT IGNORE``) so that two
845-
calls racing to create the same row do not surface a duplicate-key error. The
846-
losing call no-ops on the insert and refetches the winning row with
847-
``created=False``. ``created`` is ``True`` only for the call whose insert
810+
insert (``INSERT ... ON CONFLICT DO NOTHING`` on PostgreSQL and SQLite) so
811+
that two calls racing to create the same row do not surface a duplicate-key
812+
error. The losing call no-ops on the insert and refetches the winning row
813+
with ``created=False``. ``created`` is ``True`` only for the call whose insert
848814
actually landed.
849815
850816
:param session: The SQLAlchemy asynchronous session to use for database
851817
operations.
852-
:type session: AsyncSession
853818
:param instance_create: The data used to filter and possibly create the
854819
instance.
855-
:type instance_create: B
856820
:param filter_include: The set of fields of `instance_create` to be included in
857821
the search filter. Use None (default) for all fields.
858-
:type filter_include: set[str] | None
859822
:param extra_fields: Additional fields to be set on the created instance.
860-
:type extra_fields: Any
861823
:return: The existing or newly created instance of `cls.Model`, and a bool
862824
specifying whether a new instance was created.
863-
:rtype: tuple[T, bool]
864825
:raises HTTPBadRequestException: If a ``DatabaseError`` occurs during the
865826
insert commit.
866827
:raises RuntimeError: If the post-conflict refetch matches no row, meaning

app/core/db/utils.py

Lines changed: 7 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
text,
3434
TypeDecorator,
3535
)
36-
from sqlalchemy.dialects import mysql, postgresql, sqlite
36+
from sqlalchemy.dialects import postgresql, sqlite
3737
from sqlalchemy.dialects.postgresql import JSONB
3838
from sqlalchemy.engine import Connection
3939
from sqlalchemy.ext.asyncio import async_sessionmaker, AsyncEngine, create_async_engine
@@ -153,9 +153,6 @@ def func_json_extract(
153153
expression indexes keep matching.
154154
- SQLite: ``json_extract(col, '$.a.b')``. SQLite auto-unquotes scalars, so
155155
the result is directly comparable to a string.
156-
- MySQL: ``json_extract(col, '$.a.b')``. No functional index is created on
157-
MySQL because a width-limited ``CAST`` would introduce comparison
158-
truncation; MySQL dev environments fall back to non-indexed filtering.
159156
160157
:param db_engine: The database engine type (e.g., ``"postgresql"``).
161158
:type db_engine: str
@@ -183,39 +180,29 @@ def func_json_extract(
183180
def idempotent_insert(engine_name: str, table: Any) -> GenericInsert:
184181
"""Return a dialect-specific INSERT that ignores duplicate-key conflicts.
185182
186-
PostgreSQL and SQLite use ``INSERT ... ON CONFLICT DO NOTHING``; MySQL uses
187-
``INSERT IGNORE ...``. The caller chains ``.values(...)`` and passes the
188-
result to ``session.execute``.
183+
PostgreSQL and SQLite use ``INSERT ... ON CONFLICT DO NOTHING``. The caller
184+
chains ``.values(...)`` and passes the result to ``session.execute``.
189185
190-
:param engine_name: SQLAlchemy engine ``name`` (``"postgresql"``, ``"sqlite"``,
191-
or ``"mysql"``).
192-
:type engine_name: str
186+
:param engine_name: SQLAlchemy engine ``name`` (``"postgresql"`` or
187+
``"sqlite"``).
193188
:param table: The target table or ORM model class.
194-
:type table: Any
195189
:return: A dialect-specific insert construct.
196-
:rtype: GenericInsert
197190
:raises NotImplementedError: If the dialect is not supported.
198191
"""
199192
if engine_name == DatabaseDialect.POSTGRESQL:
200193
return postgresql.insert(table).on_conflict_do_nothing()
201194
if engine_name == DatabaseDialect.SQLITE:
202195
return sqlite.insert(table).on_conflict_do_nothing()
203-
if engine_name == DatabaseDialect.MYSQL:
204-
return mysql.insert(table).prefix_with("IGNORE")
205196
raise NotImplementedError(f"idempotent_insert: unsupported dialect {engine_name!r}")
206197

207198

208199
class NullsLastOrdering(ColumnElement):
209200
"""Render an ``ORDER BY`` term that places NULLs last on every supported dialect.
210201
211-
PostgreSQL and SQLite render the standard ``NULLS LAST`` clause. MySQL has no
212-
such syntax, so its hook prepends ``ISNULL(<expr>) ASC`` -- ``ISNULL`` yields
213-
``1`` for NULL and ``0`` otherwise, pinning NULLs last independently of the
214-
primary direction.
202+
PostgreSQL and SQLite render the standard ``NULLS LAST`` clause.
215203
216204
Takes the direction as a flag rather than a pre-directed expression: wrapping an
217-
already-``desc()``-ed expression would make the MySQL hook emit the invalid
218-
``ISNULL(<expr> DESC)``.
205+
already-``desc()``-ed expression would render ``<expr> DESC ASC NULLS LAST``.
219206
220207
Participates in SQLAlchemy's compiled-statement cache, with a key that
221208
discriminates both column and direction.
@@ -253,30 +240,6 @@ def _compile_nulls_last_ordering(
253240
return f"{compiler.process(element.column, **kw)} {direction} NULLS LAST"
254241

255242

256-
@compiles(NullsLastOrdering, DatabaseDialect.MYSQL)
257-
def _compile_nulls_last_ordering_mysql(
258-
element: NullsLastOrdering, compiler: SQLCompiler, **kw: Any
259-
) -> str:
260-
"""Render MySQL's ``ISNULL(<expr>) ASC, <expr> <direction>`` equivalent.
261-
262-
The interpolated text is the compiler's own rendering of the wrapped
263-
expression, never a client-supplied value: sort keys are allowlisted by
264-
:attr:`~app.core.db.list_query.ListQuerySpec.sortable` before they reach the
265-
construct, and :class:`NullsLastOrdering` coerces a raw string argument into a
266-
bound parameter rather than SQL text. Path literals carried by
267-
:func:`func_json_extract` use ``literal_execute``, so the dialect's literal
268-
processor inlines them at execution -- the same rendering that function
269-
documents, unchanged by the wrapper.
270-
271-
:param element: The ordering construct being compiled.
272-
:param compiler: The active SQL compiler.
273-
:return: The rendered pair of ``ORDER BY`` terms.
274-
"""
275-
rendered = compiler.process(element.column, **kw)
276-
direction = "DESC" if element.descending else "ASC"
277-
return f"ISNULL({rendered}) ASC, {rendered} {direction}"
278-
279-
280243
def prepare_unsafe_value_for_json_comparison(db_engine: str, value: Any) -> Any:
281244
"""Prepare a value for JSON comparison based on the database engine.
282245

app/core/settings_override/registry.py

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1148,10 +1148,9 @@ def _stored_key_matches_override_key(
11481148
"""Return whether a stored override key resolves to the same field as ``key``.
11491149
11501150
Nested keys go through :func:`canonical_override_key`. Top-level keys also
1151-
match case-insensitively: the previous SQL ``WHERE key = ...`` lookup
1152-
inherited MySQL's ``utf8mb4_0900_ai_ci`` collation, so moving the filter
1153-
into Python must not drop a mixed-case top-level row that DELETE/PATCH
1154-
used to find. Snapshot application still ignores unknown casing via
1151+
match case-insensitively so mixed-case stored keys remain visible to
1152+
DELETE/PATCH after the filter moved into Python. Snapshot application still
1153+
ignores unknown casing via
11551154
:func:`app.core.settings_override.cache._apply_top_level_row`; DELETE
11561155
removes those inert rows, and PATCH heals their stored key to the
11571156
canonical spelling so the next snapshot can read them.
@@ -1182,7 +1181,7 @@ async def override_rows_for_key(
11821181
non-canonically-cased nested or top-level row visible to DELETE and PATCH,
11831182
which previously matched the stored column with dialect-dependent SQL
11841183
equality and, after the filter moved into Python, missed mixed-case
1185-
top-level rows on MySQL.
1184+
top-level rows.
11861185
11871186
Inactive rows are included: both write paths currently match on
11881187
``(setting_class, key)`` alone, so an inactive row stays deletable and

0 commit comments

Comments
 (0)