mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-07-19 11:21:31 +00:00
fix(build): move appstate.py + routers/ under lib/ so the desktop app ships them (#836)
The packaged desktop app died at startup:
File ".../Resources/slopsmith/server.py", line 71, in <module>
import appstate
ModuleNotFoundError: No module named 'appstate'
feedback-desktop's scripts/bundle-slopsmith.sh copies a HARDCODED list of core
files into the bundle -- server.py, VERSION, lib/, data/, static/,
plugins/__init__.py. The root-level appstate.py (#833) and routers/ (#834)
shipped fine in Docker, passed every test, and were silently dropped from the
packaged app.
Both now live under lib/, the one core directory every packaging path already
copies wholesale -- Dockerfile `COPY lib/`, docker-compose.yml, and the desktop
bundler's `cp -r lib` -- and that all three put on sys.path (on Windows via the
embeddable-Python ._pth, where PYTHONPATH is ignored: build-windows.sh writes
`../slopsmith` and `../slopsmith/lib`). No feedback-desktop change and no new
release are needed for this to take effect.
lib/ is also the CORRECT home under Principle V, and always was once the design
settled: with the injection seam, appstate.py constructs nothing and does no
import-time IO, and a route module only builds an APIRouter. The premise that
forced root placement -- "appstate opens sqlite at import" -- stopped being true
when configure() replaced ownership. The Dockerfile / .dockerignore /
docker-compose.yml entries added for the root layout are reverted; nothing else
in core changes (git mv, so --follow survives).
tests/test_packaging.py is the guard: it walks server.py's module-level imports,
keeps the ones resolving inside this repo, and fails if any lives outside a
directory the packagers copy -- with the ModuleNotFoundError spelled out. So the
next root-level core module can't ship broken. Negative-checked: restoring
appstate.py to the root fails it; the message names the file and the four
packaging files a root module would have to teach.
(It also has to skip `origin in {"built-in","frozen"}` -- on 3.14 the frozen
stdlib reports origin="frozen", and Path("frozen").resolve() lands inside the
repo, which flagged `os` and `stat` as first-party.)
Verified: the bundler's copy replicated exactly into a temp dir and booted with
PYTHONPATH=<bundle>:<bundle>/lib -- `import server`, `import appstate`,
`import routers.audio_effects` all resolve, appstate.meta_db is server.meta_db,
143 routes. The same simulation against origin/main reproduces the production
ModuleNotFoundError. pytest 2398 passed (2348 + 50 new); route table still
identical to origin/main (paths, methods, order); docker build context reaches
lib/appstate.py and lib/routers/ with no __pycache__; native uvicorn boot smoke
serves /api/version, /api/library, the moved audio-effects router, and all three
migrated plugins' src/ graphs; eslint 0 errors.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
ebe59d3f97
commit
b3215694e7
@ -5,13 +5,6 @@
|
||||
!dockerfile
|
||||
!requirements.txt
|
||||
!server.py
|
||||
# The router seam server.py injects its singletons into (R3). Root-level Python
|
||||
# that ships in the image must be re-allowed explicitly — this file starts with
|
||||
# a blanket `*` exclusion.
|
||||
!appstate.py
|
||||
# Route modules extracted from server.py (R3).
|
||||
!routers/
|
||||
!routers/**
|
||||
!main.py
|
||||
!VERSION
|
||||
!tailwind.config.js
|
||||
|
||||
28
CHANGELOG.md
28
CHANGELOG.md
@ -7,9 +7,27 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
- **The packaged desktop app could not start (`ModuleNotFoundError: No module named
|
||||
'appstate'`).** feedback-desktop's `scripts/bundle-slopsmith.sh` copies a *hardcoded
|
||||
list* of core files into the app bundle — `server.py`, `VERSION`, `lib/`, `data/`,
|
||||
`static/`, `plugins/__init__.py`. The root-level `appstate.py` and `routers/` added in
|
||||
R3 shipped correctly in Docker and passed every test, and were then silently dropped
|
||||
from the packaged app, which died at startup. Both now live under **`lib/`** — the one
|
||||
core directory the Dockerfile (`COPY lib/`), `docker-compose.yml`, and the desktop
|
||||
bundler (`cp -r lib`) all copy wholesale, and that all three put on `sys.path` (on
|
||||
Windows via the embeddable-Python `._pth`, where `PYTHONPATH` is ignored). This needs no
|
||||
change in feedback-desktop and no new release to take effect. Placing them there is also
|
||||
correct under Principle V: with the injection seam, `appstate.py` constructs nothing and
|
||||
does no import-time IO, and a route module only builds an `APIRouter`. The
|
||||
`Dockerfile` / `.dockerignore` / `docker-compose.yml` entries added for the root layout
|
||||
are reverted. New `tests/test_packaging.py` walks `server.py`'s module-level imports and
|
||||
fails if any first-party module resolves outside a directory the packagers copy, so the
|
||||
next root-level module can't ship broken.
|
||||
|
||||
### Added
|
||||
- **`routers/` — the first extracted route module (R3).** The five audio-effects mapping
|
||||
endpoints move out of `server.py` into `routers/audio_effects.py` as a
|
||||
endpoints move out of `server.py` into `lib/routers/audio_effects.py` as a
|
||||
`fastapi.APIRouter`, mounted with `app.include_router(...)` **at the point in the file
|
||||
where they used to be defined** — FastAPI matches routes in registration order, so the
|
||||
mount site preserves it. Verified: the full 143-route table (paths, methods, *and*
|
||||
@ -19,8 +37,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
resolved at call time). This proves the seam from #833 under a real consumer, including
|
||||
the second slot. The `_demo_mode_guard` middleware still blocks all four moved write
|
||||
routes with 403, and `Query(...)` validation still 422s — both checked against a running
|
||||
server. Ships via `COPY routers/ /app/routers/` plus `!routers/` + `!routers/**` in
|
||||
`.dockerignore`. `server.py`: **9,445 → 9,386 lines**.
|
||||
server. `server.py`: **9,445 → 9,386 lines**.
|
||||
- **`appstate.py` — the router seam (R3).** Route modules moving out of `server.py`
|
||||
need `meta_db` and friends but must not `import server`, or the import graph goes
|
||||
circular the moment `server` imports them back. So `server.py` keeps *constructing*
|
||||
@ -38,10 +55,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
both a later `configure()` and `monkeypatch.setattr` — the same read-only-binding trap
|
||||
as ES `import`. `configure()` rejects an unknown slot rather than silently creating a
|
||||
global nothing reads, and the suite asserts `server` actually calls it (a seam whose
|
||||
wiring can no-op undetected is worse than no seam). Ships in the image via
|
||||
`Dockerfile` `COPY appstate.py /app/` plus a `.dockerignore` allowlist entry — that
|
||||
file opens with a blanket `*` exclusion, so root-level Python must be re-allowed
|
||||
explicitly or the build fails.
|
||||
wiring can no-op undetected is worse than no seam). Lives at `lib/appstate.py`.
|
||||
|
||||
### Changed
|
||||
- **`AudioEffectsMappingDB` moved out of `server.py` into `lib/audio_effects_db.py`
|
||||
|
||||
@ -206,10 +206,6 @@ COPY --from=tailwind-builder /build/static/tailwind.min.css /app/static/tailwind
|
||||
# when a plugin is installed at runtime (see update_manager on-install hook).
|
||||
COPY tailwind.config.js /app/tailwind.config.js
|
||||
COPY server.py /app/
|
||||
# The router seam server.py injects its singletons into (R3). Root-level, like
|
||||
# server.py, so `import appstate` resolves off PYTHONPATH=/app.
|
||||
COPY appstate.py /app/
|
||||
COPY routers/ /app/routers/
|
||||
COPY main.py /app/
|
||||
COPY VERSION /app/
|
||||
# Built-in diagnostic sloppaks seeded into DLC_DIR/diagnostics-builtin/ at scan
|
||||
|
||||
@ -11,8 +11,6 @@ services:
|
||||
# Mount source for live reload during development
|
||||
- ./static:/app/static
|
||||
- ./server.py:/app/server.py
|
||||
- ./appstate.py:/app/appstate.py
|
||||
- ./routers:/app/routers
|
||||
- ./VERSION:/app/VERSION
|
||||
- ./ug_browser.py:/app/ug_browser.py
|
||||
- ./lib:/app/lib
|
||||
|
||||
@ -47,6 +47,15 @@ quietly operating on a stand-in.
|
||||
|
||||
Slots are added here only when a router actually needs one — this is a seam,
|
||||
not a grab-bag for everything in ``server.py``.
|
||||
|
||||
**Why this lives in ``lib/`` and not the repo root.** Because it constructs
|
||||
nothing and does no import-time IO, it satisfies Principle V's rule for ``lib/``
|
||||
modules — and ``lib/`` is the only core directory every packaging path already
|
||||
copies: the Dockerfile (``COPY lib/``), ``docker-compose.yml``, and
|
||||
feedback-desktop's ``bundle-slopsmith.sh`` (``cp -r lib``). All three also put
|
||||
both the bundle root and ``lib/`` on ``sys.path``. A root-level module ships in
|
||||
Docker but is silently dropped from the packaged desktop app, whose bundler
|
||||
copies a hardcoded file list — that regression is what moved this file here.
|
||||
"""
|
||||
|
||||
# The singletons routers may read. Every name here must also be a `_SLOTS` key.
|
||||
@ -19,4 +19,13 @@ and always as a **module attribute, at call time** — never
|
||||
later ``appstate.configure()`` and ``monkeypatch.setattr``. See ``appstate.py``.
|
||||
|
||||
Dependencies flow one way: ``server -> routers -> appstate``.
|
||||
|
||||
**Why this lives under ``lib/``.** ``lib/`` is the only core directory every
|
||||
packaging path already copies wholesale — the Dockerfile (``COPY lib/``),
|
||||
``docker-compose.yml``, and feedback-desktop's ``bundle-slopsmith.sh``
|
||||
(``cp -r lib``) — and all three put it on ``sys.path``. A root-level package
|
||||
ships in Docker but is silently dropped from the packaged desktop app, whose
|
||||
bundler copies a hardcoded file list. Route modules import nothing at module
|
||||
scope beyond FastAPI and ``appstate``, so they do no import-time IO and satisfy
|
||||
Principle V's rule for ``lib/``.
|
||||
"""
|
||||
@ -68,6 +68,7 @@ from metadata_db import (
|
||||
from audio_effects_db import AudioEffectsMappingDB
|
||||
# The router seam. Imported as a module (never `from appstate import ...`) so
|
||||
# `appstate.configure(...)` below publishes into the same namespace routers read.
|
||||
# Lives in lib/ because that is the one core dir every packaging path copies.
|
||||
import appstate
|
||||
# Extracted route modules. They import `appstate`, never `server` — one-way graph.
|
||||
from routers import audio_effects
|
||||
|
||||
112
tests/test_packaging.py
Normal file
112
tests/test_packaging.py
Normal file
@ -0,0 +1,112 @@
|
||||
"""Guard: every first-party module `server.py` imports must be one the packagers copy.
|
||||
|
||||
feedback-desktop's `scripts/bundle-slopsmith.sh` copies a **hardcoded list** from
|
||||
core into the app bundle — `server.py`, `VERSION`, `lib/`, `data/`, `static/`,
|
||||
`plugins/__init__.py`. A new root-level module (say `appstate.py`) ships fine in
|
||||
Docker, imports fine under pytest, and is then *silently dropped* from the
|
||||
packaged desktop app, which dies at startup with:
|
||||
|
||||
File ".../Resources/slopsmith/server.py", line 71, in <module>
|
||||
import appstate
|
||||
ModuleNotFoundError: No module named 'appstate'
|
||||
|
||||
That shipped once. This test is why it can't ship twice: it walks `server.py`'s
|
||||
module-level imports, keeps the ones that resolve inside this repo, and asserts
|
||||
each lives under a directory every packaging path already copies wholesale.
|
||||
|
||||
If you add a first-party module for `server.py`, put it in `lib/` — the one core
|
||||
directory the Dockerfile (`COPY lib/`), `docker-compose.yml`, and the desktop
|
||||
bundler (`cp -r lib`) all copy, and that all three put on `sys.path`. If you
|
||||
genuinely need it at the repo root, you must also teach `bundle-slopsmith.sh`,
|
||||
the `Dockerfile`, `.dockerignore`, and `docker-compose.yml` about it — and then
|
||||
update `BUNDLED_ROOTS` below.
|
||||
"""
|
||||
|
||||
import ast
|
||||
import importlib.util
|
||||
import pathlib
|
||||
|
||||
import pytest
|
||||
|
||||
REPO_ROOT = pathlib.Path(__file__).resolve().parent.parent
|
||||
|
||||
# Directories every packaging path copies wholesale, plus the files copied by name.
|
||||
BUNDLED_ROOTS = ("lib", "plugins", "data", "static")
|
||||
BUNDLED_FILES = ("server.py", "main.py")
|
||||
|
||||
|
||||
def _server_toplevel_imports():
|
||||
"""Module names imported at `server.py`'s top level (not inside a function)."""
|
||||
tree = ast.parse((REPO_ROOT / "server.py").read_text())
|
||||
names = set()
|
||||
for node in tree.body: # top level only — lazy imports are fine
|
||||
if isinstance(node, ast.Import):
|
||||
names.update(a.name.split(".")[0] for a in node.names)
|
||||
elif isinstance(node, ast.ImportFrom) and node.level == 0 and node.module:
|
||||
names.add(node.module.split(".")[0])
|
||||
return sorted(names)
|
||||
|
||||
|
||||
# `spec.origin` is not always a path: built-in and frozen stdlib modules use
|
||||
# these sentinels. `Path("frozen").resolve()` would land inside the repo and
|
||||
# report `os` as first-party — so filter them before touching the filesystem.
|
||||
_NON_PATH_ORIGINS = {"built-in", "frozen", "namespace"}
|
||||
|
||||
|
||||
def _first_party_origin(name):
|
||||
"""Path of `name` if it resolves inside this repo, else None (stdlib/site-package)."""
|
||||
try:
|
||||
spec = importlib.util.find_spec(name)
|
||||
except (ImportError, ValueError):
|
||||
return None
|
||||
if spec is None:
|
||||
return None
|
||||
|
||||
if spec.origin and spec.origin not in _NON_PATH_ORIGINS:
|
||||
origin = pathlib.Path(spec.origin)
|
||||
else:
|
||||
# Namespace/frozen: fall back to the first search location, if any.
|
||||
locations = list(getattr(spec, "submodule_search_locations", None) or [])
|
||||
if not locations:
|
||||
return None
|
||||
origin = pathlib.Path(locations[0])
|
||||
|
||||
if not origin.is_absolute():
|
||||
return None # a sentinel, not a real path
|
||||
origin = origin.resolve()
|
||||
try:
|
||||
origin.relative_to(REPO_ROOT)
|
||||
except ValueError:
|
||||
return None # outside the repo → a dependency
|
||||
return origin
|
||||
|
||||
|
||||
@pytest.mark.parametrize("name", _server_toplevel_imports())
|
||||
def test_server_import_is_bundled(name):
|
||||
origin = _first_party_origin(name)
|
||||
if origin is None:
|
||||
return # stdlib or an installed dependency
|
||||
|
||||
rel = origin.relative_to(REPO_ROOT)
|
||||
if rel.as_posix() in BUNDLED_FILES or rel.parts[0] in BUNDLED_ROOTS:
|
||||
return
|
||||
|
||||
raise AssertionError(
|
||||
f"server.py imports `{name}` from {rel}, which no packager copies.\n"
|
||||
f"The desktop bundler (scripts/bundle-slopsmith.sh) copies only "
|
||||
f"{BUNDLED_FILES} and {BUNDLED_ROOTS}/, so the packaged app would die "
|
||||
f"at startup with ModuleNotFoundError: No module named '{name}'.\n"
|
||||
f"Move it under lib/, or teach bundle-slopsmith.sh + Dockerfile + "
|
||||
f".dockerignore + docker-compose.yml about it and update BUNDLED_ROOTS."
|
||||
)
|
||||
|
||||
|
||||
def test_the_seam_and_routers_live_under_lib():
|
||||
"""Pin the two that already caused a shipped break."""
|
||||
for name in ("appstate", "routers"):
|
||||
origin = _first_party_origin(name)
|
||||
assert origin is not None, f"{name} does not resolve inside the repo"
|
||||
assert origin.relative_to(REPO_ROOT).parts[0] == "lib", (
|
||||
f"{name} resolved to {origin.relative_to(REPO_ROOT)}; it must live "
|
||||
f"under lib/ or the packaged desktop app will not ship it"
|
||||
)
|
||||
Loading…
Reference in New Issue
Block a user