Repository navigation
fix(migrations): make SQLite-autogenerated revisions portable to Postgres (#342) - #406
Merged
Merged
Conversation
…342) - render Boolean server defaults as sa.false()/sa.true(), not sa.text('0'), via the render_item / process_revision_directives hooks both env.py files already call (new simple_module_db.migration_portability) - loud warning when autogenerating on SQLite while models declare expression indexes; SM026 (info) in make doctor, and a log line from check_migrations - drop_enums_if_postgres() helper for downgrades - migrations-roundtrip-pg make target + CI job (upgrade heads, downgrade base, upgrade heads, alembic check) wired into pr-checks - two revisions fixing timestamp -> timestamptz drift the round trip found - docs: review checklist in module-authoring, SM026 in diagnostic codes Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Deploying simple-module-python with
|
| Latest commit: |
dd83cd6
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b8eb3f37.simple-module-python.pages.dev |
| Branch Preview URL: | https://feat-342-migration-portabili.simple-module-python.pages.dev |
Owner
Author
|
Merge-order note: see #410 — both PRs add a revision on top of |
…tion portability Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
…tribution (review pass 1) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
Owner
Author
/ship resultsPushed 2 commits: Review (2 passes, high then medium): pass 1 found 7, fixed 3 (downgrade ops now get portable Boolean defaults + regression test; SM026 attribution limited to a module's own models; duplicate predicate removed), accepted 4 (dialect string-split duplication, QA on Postgres (scratch DB
Local CI: ruff format/check, ty, biome, tsc, file-size, untranslated-strings pass; Full report: https://claude.ai/artifact/Fcgu1rqvvY4BhRaDABJR76 |
antosubash
added a commit
that referenced
this pull request
Oct 7, 2026
…amp revision Both branched from c7f2d9a41e83; repoint down_revision so main keeps a single mainline head. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
antosubash
added a commit
that referenced
this pull request
Oct 7, 2026
…, thumbnails (#410) * feat(file_storage): public files with anonymous serving, list search/sort, thumbnails Fixes #353, fixes #352. - StoredFile.public (default false; migration uses sa.false()), set on upload (public=true) or PATCH /files/{id}; StoredFileOut gains public/public_url. - Anonymous GET /public/{id}[/{filename}] and /public/{id}/thumbnail, exempted via register_public_routes (GET only). Only public, non-deleted rows resolve; other cases are one 404. Cross-tenant lookup by id uses all_tenants(). ETag/304, Cache-Control public, nosniff, sandbox CSP; active content (HTML, SVG, JS) forced to attachment and streamed. - SecurityHeadersMiddleware keeps a CSP the response already set. - GET /files: q, content_type (exact or "image/" prefix), sort. - GET /files/{id}/thumbnail?w=: Pillow WebP, width snapped to a whitelist, pixel-budget guard, variants cached in the storage backend and dropped with the file. - Browse screen: Public badge and make public/private action. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix(file_storage): harden thumbnails (format allowlist, size/pixel caps, concurrency) and add tenant/permission tests Pillow only opens JPEG/PNG/WEBP/GIF, sniffed format must match the declared type, DecompressionBombWarning is an error, source bytes are capped while reading, decodes are bounded by a semaphore, first frame only, metadata not carried over. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix(file_storage): hold the decode slot until the worker thread finishes; stop mutating Pillow's global pixel cap Decode runs as a shielded single-flight task per variant, so a cancelled request neither frees its slot early nor lets retries multiply decodes. The semaphore is per event loop. The explicit header-only pixel budget replaces the racy process-global MAX_IMAGE_PIXELS toggle. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * refactor(optimize): dedupe 404/headers helpers, drop dead code and one-use wrappers in file_storage Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix: address code review findings (round 1, pass 1) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix: address code review findings (round 1, pass 1b) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix(file_storage): no session cookie on public responses, strict PATCH bool, 16-bit grayscale thumbnails (QA round 1) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix(file_storage): log orphaned thumbnail variant deletes (review round 1, pass 3) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * test: reconcile file_storage/branding tests with #401 and #404 on main Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix(migrations): chain file_storage public column after #406's timestamp revision Both branched from c7f2d9a41e83; repoint down_revision so main keeps a single mainline head. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * feat(file_storage): own rate-limit bucket for anonymous public file GETs With #408's shared limiter on main, public file reads shared the 120/minute anonymous default; a page embedding many images would throttle itself. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #342
Summary
render_item+process_revision_directives(the hooks bothhost/migrations/env.pyand thesmpy create-hosttemplate already call) now emitsa.false()/sa.true()instead ofsa.text('0'/'1')for Boolean columns. Logic lives in the newsimple_module_db.migration_portability.make migration; the Postgres round trip is the real gate). New diagnostic SM026 (info) inmake doctor/dev boot, andcheck_migrationslogs that SQLite cannot verify them instead of implying clean.simple_module_db.drop_enums_if_postgres(op, *names). No existing revision creates enum types (verified: nopg_typeenums after full upgrade), so none needed changing.make migrations-roundtrip-pg(upgrade heads, downgrade base, upgrade heads, alembic check onSM_MIGRATIONS_PG_URL) and amigrations-roundtrip-pgjob with a Postgres service, added topr-checksneeds.docs/module-authoring.md(boolean defaults, expression indexes, enum drops, SAEnum default = member name), SM026 in CLAUDE.md anddocs/reference/diagnostic-codes.md.Findings on existing history
ix_users_user_email_loweris already created by41cf2c53660e; no new index revision needed.sa.text("0")defaults are on Integer columns (fine on Postgres); the Boolean one already usessa.false(). No historical edits.alembic checkfail on Postgres: 8 columns created as naivetimestampwhile the model (sqlmodelUTCDateTime) is timezone-aware. Fixed with two new revisions (e5f2a8c1d7b3,f6a3b9d2e8c4, the latter on the keycloak branch), Postgres-onlyALTER ... USING col AT TIME ZONE 'UTC', no-op on SQLite.Verification
make migrations-roundtrip-pgpasses against shared Postgres on a scratch DB (fresh DB, dropped afterwards): 36 upgrades, 18 downgrades,alembic checkclean.make lintpasses. New unit tests for the render hook, expression-index detection, SM026 and the check_migrations note.https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4