Commit b300f23
authored
SEP-1431: Predictably namespace app schema names in the OpenAPI spec (#1118)
## Summary
- Replace Pydantic's module-path fallback schema names
(`app__sep__apps__backup_pg__models__BackupTaskResponse`) with explicit,
stable `<app>__<Class>` names (`backup_pg__BackupTaskResponse`) that
never shift when the installed app set changes.
- Names are assigned at generation time by `_AppNamespacedJsonSchema` (a
subclass of FastAPI's `GenerateJsonSchema`, so FastAPI's `bytes_schema`
override survives); a `namespace_app_schema_names` post-pass
disambiguates generic `PaginatedResponse[...]` wrapper collisions by the
app owning the wrapped model — no positional `___N` suffixes.
- Regenerate all committed specs, per-app snapshot goldens, and the
`@sep/api` client; update the two hand-written frontend schema refs and
the API README to the namespaced names.
## Scope notes & follow-up fixes
Two items diverge from the ticket's stated scope (post-processing rename
confined to `scripts/dump_openapi.py`). Both are deliberate; full
rationale on SEP-1431.
- **Namespacing runs on the live app, not just the dump path.** The
ticket flagged the `GenerateJsonSchema`-subclass route as the "more
version-fragile" *alternative* / out of scope, but the served specs and
the committed fixtures must agree: the live `/api/sep/openapi.json` (the
frontend codegen source) and the merged `/api/openapi.json` need the
same `<app>__<Class>` names the fixtures carry, or the SEP-1386
freshness guard and the generated client drift. So
`install_namespaced_openapi` runs on every `create_app()`. The
process-global generator swap the ticket warned about is contained under
a non-reentrant lock, restored in `finally`, and verified never-nested
(the merged spec computes each sub-app's document sequentially), so all
live generation is cooperating.
- This wiring also exposed a **`namespaced_openapi` self-deadlock**:
`install_namespaced_openapi` replaces `app.openapi` with a wrapper that
calls `_generate_namespaced` under the patch lock, and
`namespaced_openapi` drove that *same* wrapper — re-entering
`_generate_namespaced` and re-acquiring the non-reentrant lock, so every
fresh dump against an installed app (`dump_openapi.py`, the freshness
guard's subprocess) hung forever. Fixed by stashing the pre-install base
generator on `app.state` and running it directly; a bare,
never-installed app still falls back to `app.openapi`. Regenerated
`sep.json` to the served names (the other three specs were already
fresh) and updated the two inventory schema assertions.
- **The aiosqlite / `pytest-timeout` test-infra fix is
in-scope-by-necessity, not ride-along.** It was surfaced *by* this
ticket: the freshness guard runs `dump_openapi.py` in a subprocess, and
wiring namespacing through that path exposed a latent hang. Root cause:
async-DB fixtures created `sqlite+aiosqlite` `AsyncEngine`s with
`StaticPool` but never disposed them — aiosqlite runs each connection on
a *non-daemon* thread, so leaked engines survived the pytest session and
blocked CPython's interpreter-exit join indefinitely, hanging `make
test` at shutdown. Disposed the engine in every leaking fixture's
teardown (matching the existing `postgres_engine` pattern) and added
`pytest-timeout` (120s) so a future setup/teardown hang fails loudly
instead of blocking. The namespacing tests cannot return without the
dispose fix, so this PR could not be verified or merged green without it
— hence no separate ticket; splitting would leave this PR unmergeable
with no way to validate the split.
## Tested
- [x] `pytest tests/app/core/utils/test_openapi.py` — namespacing unit +
generic-wrapper/collision edge cases
- [x] `pytest tests/app/test_openapi_specs_fresh.py
tests/app/sep/test_openapi_snapshot.py
tests/app/sep/test_schema_snapshot.py`
- [x] `python scripts/dump_openapi.py --check` — exit 0 (idempotent;
only `sep.json` changes)
- [x] `make test` — 7303 passed, 383 skipped, 0 failed in ~160s
(previously never returned)
- [x] `pnpm type-check` — clean across all frontend packages
- [x] `ruff check` + `ruff format --check` clean
## Checklist
- [x] New/modified functions have type hints and rST docstrings
- [x] New tests added for new features or bug fixes
- [x] All tests pass locally (`make test`)
- [x] Pre-commit hooks pass (`make run-pre-commit`)
- [x] Database migrations generated if models changed (`make
makemigrations`) — N/A, no model changes
- [x] User-facing changes documented (README, inline help, UI text)
- [x] Configuration changes documented with examples — N/A
- [x] Changelog fragment added under `changelog.d/`
> [!NOTE]
> Branch is one commit behind `origin/main`; SEP-1499 overlaps the
generated spec/client and the checksums snapshot. Rebase and regenerate
(`make regen-specs` + `pnpm --filter @sep/api codegen`) before merge.1 parent 9eb822e commit b300f23
58 files changed
Lines changed: 25488 additions & 24568 deletions
File tree
- app/core
- utils
- changelog.d
- frontend/packages
- api
- specs
- src/generated
- framework/src
- components/SchemaDrivenApp
- hooks
- scripts
- tests/app
- core
- db
- settings_override
- api
- utils
- inventory
- sep
- api/routes
- apps
- alert_troubleshooting
- alerts
- framework
- inventory
- snippets
- db
- snapshots/openapi
- tasks
- periodic
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
75 | 75 | | |
76 | 76 | | |
77 | 77 | | |
78 | | - | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
79 | 82 | | |
80 | 83 | | |
81 | 84 | | |
| |||
750 | 753 | | |
751 | 754 | | |
752 | 755 | | |
| 756 | + | |
753 | 757 | | |
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
156 | 156 | | |
157 | 157 | | |
158 | 158 | | |
159 | | - | |
| 159 | + | |
160 | 160 | | |
161 | 161 | | |
162 | 162 | | |
| |||
0 commit comments