Skip to content

Commit 90e69f1

Browse files
committed
fix: S3 streaming + Range, task heartbeat + after-commit enqueue, token rehash, scaffold make kill + i18n keys
- file_storage: stream S3 reads, get_range on backends, 206/416 Range support on downloads; S3 get() is awaited like filesystem (#453) - background_tasks: heartbeat() API + per-task stuck_after (#451); retries publish after commit, enqueue_after_commit helper (#452) - users: /auth/token persists the upgraded password hash (#457) - hosting/cli: regenerate i18n keys into the installed npm package in scaffolded apps, smpy host gen-i18n (#449) - cli: scaffolded make kill stops only this app's ports (#461) Closes #449 Closes #451 Closes #452 Closes #453 Closes #457 Closes #461 Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
1 parent 17573d7 commit 90e69f1

39 files changed

Lines changed: 1699 additions & 132 deletions

‎CHANGELOG.md‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,19 @@ All notable changes to this project are documented in this file. The format is b
1212
## [Unreleased]
1313

1414
### Added
15+
- `from background_tasks import heartbeat` (#451): a long-running task calls
16+
`heartbeat()` to refresh `heartbeat_at` (throttled to one write per 30 s; a
17+
no-op outside a task). `@shared_task(..., stuck_after=<seconds>)` sets a
18+
per-task stuck threshold that the sweep honours ahead of
19+
`SM_BG_TASKS_STUCK_AFTER_SECONDS`. No migration.
20+
- `from background_tasks import enqueue_after_commit` (#452): publishes a task
21+
only once the caller's transaction commits (nothing on rollback), and
22+
returns the reserved task id so a row can name it up front.
23+
- HTTP `Range` support on file downloads (#453): single ranges answer `206`
24+
(`416` when unsatisfiable, multi-range falls back to `200`), honouring
25+
`If-Range`. Storage backends gain `get_range(key, start, end)` (inclusive
26+
end); a third-party backend without it keeps serving full `200`s.
27+
- `smpy host gen-i18n` and a scaffold `make gen-i18n` target (#449).
1528
- **Host override layer for module copy** (#415). A host ships
1629
`host/locales/overrides/<lang>.json` (nested or flat dotted keys) and it is
1730
applied after every module, framework and host catalog, so the server-side
@@ -166,6 +179,22 @@ All notable changes to this project are documented in this file. The format is b
166179
`tenants` resolver it selects among the user's own memberships.
167180

168181
### Fixed
182+
- `S3Backend.get` streams objects in 64 KiB reads instead of buffering the
183+
whole object in memory (#453). It is now awaited like the filesystem
184+
backend, which also fixes S3 thumbnails and the streamed public-file path
185+
(they raised `TypeError`), and a missing object answers `404`, not `500`.
186+
- Admin task retries publish to the broker only after the new
187+
`TaskExecution` rows commit (#452); a rolled-back request no longer leaves a
188+
queued task with no execution row.
189+
- `POST /api/users/auth/token` persists the upgraded password hash, as the
190+
browser login already did (#457), so bcrypt hashes imported from another
191+
system migrate to Argon2 on API/mobile sign-in too.
192+
- Scaffolded apps regenerate `keys.generated.ts` in development (#449): when
193+
`packages/i18n/src` is absent the host writes into the installed
194+
`@simple-module-py/i18n` package, merging the keys it already ships.
195+
- Scaffolded `make kill` stops only this app's ports (`API_PORT`, and
196+
`VITE_PORT` read from `SM_VITE_DEV_URL`) instead of `pkill`-ing every
197+
uvicorn and vite on the machine, vitest runs included (#461).
169198
- `render_item` renders sqlmodel>=0.0.48's `UTCDateTime` as
170199
`sa.DateTime(timezone=True)` and adds `import sqlmodel` for any other
171200
sqlmodel type, so an initial autogenerate no longer emits a migration that

‎docs/framework/i18n.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,8 @@ The detection logic lives in `scripts/lib/untranslated-strings.mjs` behind its o
283283

284284
`packages/i18n/src/keys.generated.ts` is generated over every **installed** module, not just the ones the running host activated, and `make ci-js-typecheck` runs `tsc -p` for each `modules/*/tsconfig.json` and `packages/*/tsconfig.json` in the workspace. (A module shipping `.tsx` with no `tsconfig.json` fails the target outright, rather than being silently skipped — the same gap `SM017` reports.)
285285

286+
**Scaffolded apps.** There is no `packages/i18n` checkout in a scaffolded app: `@simple-module-py/i18n` is an npm dependency. When `packages/i18n/src` is absent, development boot (and `make gen-i18n` / `smpy host gen-i18n`) writes `keys.generated.ts` and `generated-resources.ts` into the nearest `node_modules/@simple-module-py/i18n/src` found by walking up from the project root (so a hoisted npm-workspace install is found; a flat host also checks `client_app/node_modules`). The shared `ui.*` and framework keys come from catalogs inside `simple_module_hosting`, and any keys already in the published file are kept (the published catalog covers modules this app does not install, so they are not treated as removals). `npm install` restores the published file; `make install` regenerates afterwards. With neither location present (a wheel-installed production app) boot skips silently; `make gen-i18n` fails loudly.
287+
286288
That distinction is load-bearing: with a filtered union, translating an inactive auth provider's pages (say `keycloak`, when this host runs `users`) would break its build on the next regeneration while the app itself ran fine. The runtime registry stays filtered — an inactive module's strings are typed, never served.
287289

288290
`t()` needs literal keys to typecheck; a key assembled at runtime won't resolve.

‎docs/guide/installation.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ Pass `--loosen` to rewrite every constraint to `>=<latest>` instead.
165165
Run `uv tool update-shell` (or restart your shell) so the tool's bin dir is on PATH.
166166

167167
**Port already in use.**
168-
Free `:8000` and `:5050` before the next `make dev`. On Linux/macOS: `lsof -ti:8000,5050 | xargs kill -9`.
168+
Run `make kill` to free the app's API and Vite ports (`:8000` and the port in `SM_VITE_DEV_URL`, default `:5050`) before the next `make dev`. It only touches those ports, never other apps' servers. To run two apps side by side, give each its own `API_PORT` (`make dev API_PORT=8010`) and `SM_VITE_DEV_URL`.
169169

170170
**Alembic complains about a revision mismatch.**
171171
The DB is ahead of or behind the migration files. For a dev DB: `rm app.db && make migrate`. For Postgres: `docker compose down -v && docker compose up -d postgres && make migrate`.

‎docs/modules/background_tasks.md‎

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,63 @@ send_receipt.delay(order_id=42)
3232

3333
Every dispatch / start / completion / failure / retry / revoke is recorded in the `background_tasks_task_execution` table by Celery signal handlers, so you can see history even after the Celery result backend has expired the result.
3434

35+
## Enqueueing from inside a transaction
36+
37+
`send_receipt.delay(...)` publishes *now*. If the request that called it is still writing the row the task will read, two things go wrong: a fast worker (or eager mode, which runs the task inside the publish) starts before the row is visible, and if the commit then fails the task is queued against a row that never existed. Publish after commit instead:
38+
39+
```python
40+
from background_tasks import enqueue_after_commit
41+
42+
43+
async def place(self, data: OrderCreate) -> OrderOut:
44+
order = Order(**data.model_dump())
45+
self.db.add(order)
46+
await self.db.flush()
47+
enqueue_after_commit(self.db, send_receipt, order.id)
48+
return OrderOut.model_validate(order)
49+
```
50+
51+
`enqueue_after_commit(db, task, *args, **kwargs)` sends `task` once `db`'s transaction commits and drops it if the transaction rolls back or the commit fails. It returns the Celery task id the message will carry, reserved up front, so you can store it on your own row in the same transaction.
52+
53+
- `task` is a task (`send_receipt`), a signature for routing options (`send_receipt.s().set(queue="mail", countdown=30)`), or a registered task name (`"orders.send_receipt"`, sent through `celery.current_app`).
54+
- In a request (`get_db`), the publish runs from the session's `on_commit` hook — after the commit, before the response leaves.
55+
- Any other session — a task body's sync session, a CLI script's own `async with session.begin()` — hangs it off SQLAlchemy's `after_commit` event instead, and drops it when the outermost transaction ends without committing. Publishing then blocks the committing thread for the length of one kombu publish.
56+
- The task runs as the tenant bound when you called `enqueue_after_commit`, not whatever is bound at commit time.
57+
- A publish that fails after the commit is logged, not raised: the transaction is already durable.
58+
59+
The admin retry buttons follow the same rule — the new `pending` row commits first, then the task is sent (GH #452). An orphaned `pending` row that was never sent is visible and retryable; a task with no row is neither.
60+
61+
## Long-running tasks: heartbeats and stuck thresholds
62+
63+
The beat sweep flips a `running` row to `stuck` once its `heartbeat_at` is older than the stuck threshold (`stuck_after_seconds`, default 300), which makes it retry-eligible. The signal handlers only stamp the heartbeat when a task starts and finishes, so a task that runs longer than the threshold must say it is still alive, or it can be retried — and run twice — while the first run is still working.
64+
65+
Call `heartbeat()` from the task body's loop:
66+
67+
```python
68+
from background_tasks import heartbeat
69+
from celery import shared_task
70+
71+
72+
@shared_task(name="reports.rebuild")
73+
def rebuild(report_id: int) -> None:
74+
for chunk in chunks(report_id):
75+
process(chunk)
76+
heartbeat()
77+
```
78+
79+
It refreshes `heartbeat_at` on the current execution's row (found through Celery's `current_task`), writing at most once every 30 seconds per task however often you call it, so calling it on every iteration is fine; `heartbeat(force=True)` skips the throttle. Outside a running task it does nothing and returns `False`, so shared helpers can call it unconditionally. It only touches a row that is still `running` — a heartbeat never revives a row the sweep already declared stuck.
80+
81+
A task whose steps are legitimately longer than the global threshold can declare its own, in seconds:
82+
83+
```python
84+
@shared_task(name="exports.full_dump", stuck_after=4 * 3600)
85+
def full_dump() -> None: ...
86+
```
87+
88+
Celery copies extra decorator options onto the task class; the sweep reads `stuck_after` off the task registry of the worker running it (which autodiscovers every module's tasks), and falls back to `SM_BG_TASKS_STUCK_AFTER_SECONDS` / the default for tasks without one. No column or migration is involved, so the threshold always matches the deployed code. A shorter `stuck_after` works too, for tasks that should never take long. Keep any threshold well above 30 seconds, or the heartbeat throttle alone can let a live task look stale.
89+
90+
Not provided: an automatic heartbeat thread. A task that blocks in one long call (a single huge query, an external process) cannot heartbeat from inside it — give it a `stuck_after` that covers the call instead.
91+
3592
## Scheduling periodic work (beat)
3693

3794
A module schedules recurring work the same way it registers tasks — by shipping a `tasks.py` — and additionally exporting a module-level `BEAT_SCHEDULE` dict. `build_celery` merges every installed module's `BEAT_SCHEDULE` into the beat schedule at boot (identically in the web and worker processes), so `make beat` runs them alongside the built-ins:
@@ -145,7 +202,7 @@ DB-backed; defaults are in `BackgroundTasksSettings`. Several are marked `requir
145202
| `task_default_queue` | `"default"` | restart required |
146203
| `task_always_eager` | `False` | runs tasks in-process (test mode) |
147204
| `task_eager_propagates` | `True` | re-raises eager-mode errors |
148-
| `stuck_after_seconds` | `300` | heartbeat-staleness threshold |
205+
| `stuck_after_seconds` | `300` | heartbeat-staleness threshold; a task's own `stuck_after` overrides it — see [Long-running tasks](#long-running-tasks-heartbeats-and-stuck-thresholds) |
149206
| `stuck_sweep_interval_seconds` | `60` | beat cadence for the stuck sweep |
150207
| `purge_interval_seconds` | `86_400` | beat cadence for old-row purge |
151208
| `retention_days` | `14` | how long to keep terminal rows; range 1-3650 |
@@ -170,7 +227,7 @@ Bootstrap env-var equivalents (`SM_BG_TASKS_*`) only seed pydantic defaults at f
170227

171228
| Task | Schedule | What |
172229
|---|---|---|
173-
| `background_tasks.sweep_stuck_tasks` | every `stuck_sweep_interval_seconds` | flips `running` rows with stale heartbeats to `stuck` |
230+
| `background_tasks.sweep_stuck_tasks` | every `stuck_sweep_interval_seconds` | flips `running` rows whose heartbeat is older than their task's `stuck_after` (else `stuck_after_seconds`) to `stuck` |
174231
| `background_tasks.purge_old_executions` | every `purge_interval_seconds` | deletes terminal rows older than `retention_days` |
175232

176233
A `background_tasks.demo_echo` task is also registered for smoke tests; not scheduled.
@@ -202,7 +259,7 @@ Synchronous handlers in `signals.py`, all bound through Celery's `signals.<name>
202259
|---|---|
203260
| `before_task_publish` | inserts a `pending` row |
204261
| `task_prerun` | flips to `running`, stamps `started_at` + `heartbeat_at` |
205-
| `task_postrun` | refreshes `heartbeat_at` |
262+
| `task_postrun` | refreshes `heartbeat_at` (between prerun and postrun, only `heartbeat()` from the task body does) |
206263
| `task_success` | flips to `success`, stores `result`, sets `finished_at` |
207264
| `task_failure` | flips to `failed`, stores `traceback` + `exception_type`, publishes `TaskFailed` |
208265
| `task_retry` | flips to `retrying`, increments `retries` |

‎docs/modules/file_storage.md‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ Pluggable file storage with two shipped backends — local filesystem and S3-com
2222
| `GET /api/file-storage/files/{file_id}` | → `StoredFileOut` | `file_storage.download` |
2323
| `PATCH /api/file-storage/files/{file_id}` | `{"public": bool}` → `StoredFileOut` | `file_storage.upload` |
2424
| `GET /api/file-storage/files/{file_id}/thumbnail` | `?w=` → `image/webp` | `file_storage.download` |
25-
| `GET /api/file-storage/files/{file_id}/download` | → 302 (S3) or stream (filesystem) | `file_storage.download` |
25+
| `GET /api/file-storage/files/{file_id}/download` | → 302 (S3) or stream (filesystem); honours `Range` (see [Range requests](#range-requests)) | `file_storage.download` |
2626
| `DELETE /api/file-storage/files/{file_id}` | → 204 | `file_storage.delete` |
2727
| `GET /api/file-storage/public/{file_id}[/{filename}]` | bytes (**anonymous**) | none, `public` files only |
2828
| `GET /api/file-storage/public/{file_id}/thumbnail` | `?w=` → `image/webp` (**anonymous**) | none, `public` files only |
@@ -85,6 +85,29 @@ Serving rules:
8585
- Presigning backends (S3) answer with a `302` to the presigned URL, cached
8686
for at most half the signature's TTL; filesystem backends stream.
8787

88+
### Range requests
89+
90+
Whenever bytes stream through the app (the download endpoint on a
91+
non-presigning backend, and the public route for filesystem backends or active
92+
content), a single `Range` is honoured so COG / PMTiles readers can fetch
93+
just the tiles they need:
94+
95+
- `bytes=a-b`, `bytes=a-` and the suffix form `bytes=-n` answer `206 Partial
96+
Content` with `Content-Range: bytes a-b/size` and the slice's
97+
`Content-Length`. An end past the object is clamped to `size - 1`.
98+
- A range starting at or past the end (or `bytes=-0`) is `416` with
99+
`Content-Range: bytes */size`.
100+
- Multiple ranges, another unit, or a malformed header are ignored and the
101+
whole file is sent with `200` (RFC 9110 allows this). So is a request whose
102+
`If-Range` does not match the current `ETag`.
103+
- Streamed responses advertise `Accept-Ranges: bytes`. Auth, `ETag`,
104+
`Content-Disposition`, `Cache-Control` and CSP headers are the same as for
105+
the full response.
106+
107+
Redirected (presigned) downloads are served by the object store, which handles
108+
`Range` itself. Always redirecting large downloads to a presigned URL, as an
109+
opt-in setting, is not implemented.
110+
88111
### View
89112

90113
| Method + path | Inertia component | Permission |
@@ -229,12 +252,19 @@ class GcsBackend(StorageBackend):
229252
supports_presigned_url = True
230253

231254
async def put(self, key, stream, content_type, size): ...
232-
async def get(self, key): ...
255+
async def get(self, key): ... # awaited; returns an AsyncIterator[bytes]
256+
async def get_range(self, key, start, end): ... # end inclusive
233257
async def delete(self, key): ...
234258
async def exists(self, key) -> bool: ...
235259
async def presigned_get_url(self, key, ttl_seconds: int) -> str: ...
236260
```
237261

262+
`get` and `get_range` must stream in bounded chunks (never buffer the whole
263+
object) and raise `StorageNotFoundError` from the awaited call, before the
264+
first byte. `get_range(key, start, end)` uses HTTP semantics — `end` is
265+
inclusive — and receives an already-validated `0 <= start <= end < size`. A
266+
backend without `get_range` still works: the endpoints then ignore `Range`.
267+
238268
Register it in your module's `register_settings` so the file_storage service can pick it up. (The shipped two backends are wired into a private `BACKEND_REGISTRY`; see `backends/__init__.py` for the pattern.)
239269

240270
## Permissions

‎docs/modules/users.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,17 @@ Built-in provider keys (the `{provider}` URL segment):
384384

385385
A single dispatcher pair (`/api/users/auth/{provider}/login` + `/callback`) serves every provider; the client is resolved per request from the cache. The `/callback` returns a 303 redirect (not the stock fastapi-users 204) so Inertia lands on a real page, with the auth cookie attached. Find-or-create + email association goes through `UserManager.oauth_callback` (`associate_by_email=True`, `is_verified_by_default=True`); state CSRF uses the signed session cookie. Linked accounts are stored in `users_oauth_account`. A **newly provisioned** OAuth account is marked external (see below); an existing password account linked by email keeps its password and is left unchanged.
386386

387+
## Importing users from another system
388+
389+
`users` stores passwords through fastapi-users' default `PasswordHelper`, so you can import `hashed_password` values from another system and users keep their existing passwords (no reset needed). Accepted formats:
390+
391+
| Format | Prefix | Role |
392+
|---|---|---|
393+
| Argon2 (`argon2id`/`argon2i`/`argon2d`, PHC string) | `$argon2...` | Current scheme, used for all new hashes |
394+
| bcrypt | `$2a$` / `$2b$` / `$2y$` | Accepted on import (deprecated) |
395+
396+
Other formats (PBKDF2, scrypt, SHA-based, plain text) are **not** recognised, and such users cannot sign in until they reset their password. A bcrypt hash is verified as-is and transparently re-hashed to Argon2 on the user's next successful sign-in, through both the session login (`/api/users/auth/login`) and the bearer-token login (`/api/users/auth/token`). Leave `hashed_password` as `NULL` for accounts that should only sign in through an external IdP (see below).
397+
387398
## External / SSO users
388399

389400
Users created through an external IdP (Google, GitHub, Microsoft/Entra ID, or generic OIDC) are provisioned with `is_external=True` and **no local password** (`hashed_password` is `NULL`). The OAuth callback's find-or-create only nulls the password + marks external for accounts it *creates* — an existing password account that gets linked by email keeps its password and stays non-external. External users sign in **only** through their IdP; roles are still assigned locally like any other user.

0 commit comments

Comments
 (0)