Skip to content

Commit 098a92c

Browse files
authored
fix(hosting): wizard migrates the app's database; keep explicit bootstrap fields; one dotenv source; bounded Redis leases (#803)
* fix(hosting): migrate the app's database; keep explicit bootstrap fields; one dotenv source; bounded Redis leases - setup wizard passes the app engine URL to Alembic (`sm.database_url` attribute); host and scaffolded env.py prefer it over Settings(), so the migrated database is the one the lock covers (#782) - merge_host_settings(bootstrap) preserves every field set on the bootstrap (#783) - HostSettings discovers the project .env like BootstrapSettings, so boot and runtime reads agree without load_dotenv_into_environ() (#793) - lease acquire bounds each backend call by the remaining wait (1 s floor); Redis client gets 2 s socket timeouts and no hidden retries; acquire timeouts/connection failures return False, release failures log (#787) Claude-Session: https://claude.ai/code/session_01FXHA3ZicrtZ4C5QPEgxfgq * docs(leases): 1 s minimum attempt budget Claude-Session: https://claude.ai/code/session_01FXHA3ZicrtZ4C5QPEgxfgq * refactor(optimize): fold an import; one-line Alembic URL fallback Claude-Session: https://claude.ai/code/session_01FXHA3ZicrtZ4C5QPEgxfgq * fix: address code review findings (round 1, pass 1) Mark the .env precedence and lease acquisition entries as behaviour changes. Claude-Session: https://claude.ai/code/session_01FXHA3ZicrtZ4C5QPEgxfgq * fix: address code review findings (round 1, pass 2) - one immediate Redis reconnect so a stale pooled connection is not an outage - ALEMBIC_DATABASE_URL_ATTRIBUTE constant shared by the wizard and env.py - leases docs: only timeouts and Redis connection failures return False - CHANGELOG: upgrade note for hosts scaffolded before #782 - lease tests: tighter limit assertions, looser wall-clock bounds Claude-Session: https://claude.ai/code/session_01FXHA3ZicrtZ4C5QPEgxfgq * docs(i18n): runtime HostSettings resolution order Claude-Session: https://claude.ai/code/session_01FXHA3ZicrtZ4C5QPEgxfgq * docs(leases): worst-case Redis call time Claude-Session: https://claude.ai/code/session_01FXHA3ZicrtZ4C5QPEgxfgq * test: mark the simulated-latency sleeps as intentional Claude-Session: https://claude.ai/code/session_01FXHA3ZicrtZ4C5QPEgxfgq
1 parent a7c7db7 commit 098a92c

16 files changed

Lines changed: 478 additions & 43 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,21 @@ All notable changes to this project are documented in this file. The format is b
101101
- Hosting: readiness and setup connection probes have a five-second deadline
102102
per check. Timeouts report "timed out"; raised exceptions are logged and
103103
return only their class name, including database connection failures (#786).
104+
- Setup migrations lock and migrate the app's resolved database, including in
105+
newly scaffolded hosts, when it differs from `SM_DATABASE_URL` (#782). Hosts
106+
scaffolded earlier keep their own `migrations/env.py`: read
107+
`config.attributes.get(ALEMBIC_DATABASE_URL_ATTRIBUTE)` (from
108+
`simple_module_hosting.migrations`) before falling back to `Settings()`.
109+
- Explicit bootstrap process settings survive the pre-app settings merge,
110+
including production environment, module selection and Redis (#783).
111+
- **Behaviour change** (`hosting`): runtime host settings discover the same
112+
project `.env` as boot settings; a field set there now pins the value over a
113+
stored DB override and refuses admin-UI edits, like a process env var (#793).
114+
- **Behaviour change** (`core`, `hosting`): lease and semaphore acquisition
115+
bounds backend calls by the wait deadline; Redis uses finite socket timeouts
116+
without retries, and connectivity failures return not-acquired or log failed
117+
release without masking body errors (#787); `wait=0` now allows one attempt up
118+
to 1 s.
104119
- **Behaviour change** (`auth`, `oidc`): a generic-OIDC browser session created
105120
before #750 (no authority marker) is adopted only when the issuer recorded in
106121
its `oidc_identity` matches the discovered issuer (#764). A session from a

‎docs/framework/i18n.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ A pydantic validator enforces that the default locale is in the supported list.
214214
`HostSettings` is consumed through **two different objects**, and i18n uses the env-backed one:
215215

216216
- `Settings` (`HostSettings` + `BootstrapSettings`) inherits `env_prefix="SM_"`, so every field on it resolves from `SM_*` at boot. This instance lands on `app.state.sm.settings`, and it is what `LocaleMiddleware`, the i18n manifest, the shared-props builder and `i18n_deps` all read.
217-
- `app.state.host.settings` is a separate `HostSettings` hydrated from the DB. `HostSettings` on its own declares no `env_prefix`, so this instance is pydantic defaults plus stored overrides. `maintenance_mode` is read from here, which is what makes that flag live-editable.
217+
- `app.state.host.settings` is a separate `HostSettings` hydrated from the DB. It resolves env (process variables, then the project `.env`) → stored overrides → defaults, so an admin-UI edit lands here unless env pins the field. `maintenance_mode` is read from here, which is what makes that flag live-editable.
218218

219219
The practical consequence: **changing the i18n values in the admin UI does not change locale resolution.** `LocaleMiddleware` captured its locale set at construction from the env-derived object, and a settings save swaps `app.state.host.settings`, not `app.state.sm.settings`. Moving the served locale set means setting the env var and restarting. The same is true of `multi_tenant` / `tenant_header` — `SM_MULTI_TENANT` is what decides whether `TenantMiddleware` is installed at all, and no DB write can install a middleware after boot.
220220

‎docs/framework/leases.md‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,18 @@ The backend is process-global because a Celery worker has no `app.state`. `creat
5353
| a database, no Redis | `DbLeaseBackend` (`simple_module_db`) | Rows in `sm_lease (name, slot, holder, fence, expires_at)`, primary key `(name, slot)`. Every comparison uses the database clock, so clock skew between hosts does not matter; on Postgres the clock is read after the slot row is locked, so a lock wait never shortens a TTL. |
5454
| neither | `InProcessLeaseBackend` | One process only. Also what tests and eager Celery get when nothing else is configured. |
5555

56-
Backend errors propagate. A lease that the backend could not confirm is never reported as held. This is unlike the rate limiter, which falls back to per-process counters.
56+
Acquisition timeouts, and Redis connection failures, log a WARNING and return
57+
`False` (other backends' errors, such as a database outage under
58+
`DbLeaseBackend`, still propagate); `async with` consequently raises `LeaseNotAcquiredError`. An explicit
59+
`wait` bounds backend calls as well as polling, with a 1-second minimum attempt
60+
budget (including `wait=0`). Redis connections and socket reads each have a
61+
2-second timeout and at most one immediate reconnect (so roughly 4 s at
62+
worst against an unresponsive server), which also bounds calls when no
63+
`wait` is supplied. A Redis release timeout or connection failure logs a
64+
WARNING and returns `False`, so cleanup preserves an exception from the lease
65+
body; the remote holder expires by TTL. Refresh errors and other backend
66+
errors still propagate. There is no fallback to a local lease: a lease that
67+
the backend could not confirm is never reported as held.
5768

5869
Override the backend with `configure_leases(backend)`, or pass `backend=` to a single `lease()`/`semaphore()` call. `configure_leases(None)` restores a fresh in-process backend, which is useful in test teardown.
5970

‎docs/framework/settings.md‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,21 @@ class BootstrapSettings(BaseSettings):
2929

3030
The `SM_` prefix means every field is set via `SM_<FIELD>` (e.g. `SM_DATABASE_URL`, `SM_LOG_FORMAT`). `multi_tenant` / `tenant_header` and the other tunables live on the DB-backed `HostSettings` half.
3131

32+
Hosts call `merge_host_settings()` before `create_app` to apply stored host
33+
settings before middleware and module discovery are built. Precedence is
34+
**env → DB → default**. Here, env includes the discovered project `.env`;
35+
process environment variables take precedence over that file. Both the boot
36+
`Settings` and runtime `HostSettings` discover the same file via
37+
`find_env_file()`, including from a project subdirectory, without requiring
38+
`load_dotenv_into_environ()` first. Direct settings construction supports an
39+
explicit `_env_file` path or `_env_file=None` to disable dotenv loading.
40+
41+
When passing a `BootstrapSettings` to `merge_host_settings(bootstrap)`, every
42+
field explicitly supplied on that object is preserved, including `environment`,
43+
`modules_enabled`, `redis_url`, `debug` and `vite_dev_url`. The resolved secret
44+
key and database URL are preserved too. Fields the bootstrap did not supply
45+
still use env → DB → default.
46+
3247
Construct once at boot, put on `app.state.sm.settings`, never mutated.
3348

3449
### Placeholder-secret check

‎docs/reference/env-vars.md‎

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -53,26 +53,27 @@ Pools are **per process**. With multiple `uvicorn --workers`, total connections
5353

5454
## Host settings (`HostSettings`)
5555

56-
These are declared on `HostSettings` and registered under `package="host"`, so they appear in the admin UI at `/admin/settings/`. They are **also** readable from env: `Settings` combines `HostSettings` with `BootstrapSettings` and inherits its `env_prefix="SM_"`, so each field below resolves from the matching `SM_*` variable at boot.
57-
58-
Which source actually wins depends on the field, because `HostSettings` is consumed through two different objects:
59-
60-
| Setting | Env var | Default | Read from | Notes |
61-
|---|---|---|---|---|
62-
| `multi_tenant` | `SM_MULTI_TENANT` | `false` | **env at boot** | Decides whether `TenantMiddleware` is installed. A DB write cannot install a middleware after boot. |
63-
| `tenant_header` | `SM_TENANT_HEADER` | `""` | **env at boot** | Header identifying the tenant (empty = header lookup disabled). |
64-
| `i18n_default_locale` | `SM_I18N_DEFAULT_LOCALE` | `en` | **env at boot** | Must be in `i18n_supported_locales`. |
65-
| `i18n_supported_locales` | `SM_I18N_SUPPORTED_LOCALES` | `["en"]` | **env at boot** | JSON array, e.g. `["en","es"]`. |
66-
| `i18n_cookie_name` | `SM_I18N_COOKIE_NAME` | `locale` | **env at boot** | Cookie storing the selected locale. |
67-
| `maintenance_mode` | *(none — see below)* | `false` | **DB at request time** | Serve everyone but admins a 503. DB-backed so flipping it needs no redeploy — see [Deployment](/reference/deployment#maintenance-mode). |
68-
| `maintenance_message` | *(none — see below)* | `""` | **DB at request time** | Optional operator note on the maintenance page. |
69-
| `public_base_url` | `SM_PUBLIC_BASE_URL` | `""` | **DB at request time** | Public origin (`https://example.org`) for canonical / `og:url` in the server-rendered head. Never taken from the request `Host`. See [Inertia](/frontend/inertia#seo-head). |
70-
71-
The split is not arbitrary. The boot instance (`Settings()`, env-derived) lands on `app.state.sm.settings` and is what configures middleware at construction — `LocaleMiddleware` captures its locale set there, and the tenancy flag decides whether `TenantMiddleware` is added at all. The DB-hydrated instance is a plain `HostSettings` on `app.state.host.settings`, which declares no `env_prefix` and so is defaults-plus-overrides; `MaintenanceMiddleware` reads that one on every request.
72-
73-
So editing the tenancy or i18n rows in the admin UI updates what the UI shows without moving what the app serves — those need the env var and a restart. Maintenance mode is the one that genuinely takes effect live.
74-
75-
The two maintenance fields have **no working env var**, and this is worth stating plainly because the shape of the code suggests otherwise. `SM_MAINTENANCE_MODE=true` does set `maintenance_mode` on the boot `Settings()` object — but nothing reads maintenance from there, and the DB-hydrated `HostSettings` that `MaintenanceMiddleware` does read carries no `SM_` prefix, so it never sees the variable. Setting it looks plausible and silently does nothing. Flip the flag in the admin UI, or write the settings row directly.
56+
These are declared on `HostSettings` and registered under `package="host"`, so
57+
they appear in the admin UI at `/admin/settings/`. Both boot `Settings` and
58+
runtime `HostSettings` resolve **env → DB → default**, where env includes the
59+
discovered project `.env` and process variables take precedence over that file.
60+
61+
| Setting | Env var | Default | Applied |
62+
|---|---|---|---|
63+
| `multi_tenant` | `SM_MULTI_TENANT` | `false` | At boot; restart required. |
64+
| `tenant_header` | `SM_TENANT_HEADER` | `""` | At boot; restart required. |
65+
| `i18n_default_locale` | `SM_I18N_DEFAULT_LOCALE` | `en` | At boot; must be in supported locales. Restart required. |
66+
| `i18n_supported_locales` | `SM_I18N_SUPPORTED_LOCALES` | `["en"]` | At boot; restart required. |
67+
| `i18n_cookie_name` | `SM_I18N_COOKIE_NAME` | `locale` | At boot; restart required. |
68+
| `maintenance_mode` | `SM_MAINTENANCE_MODE` | `false` | Runtime; serve everyone but admins a 503. |
69+
| `maintenance_message` | `SM_MAINTENANCE_MESSAGE` | `""` | Runtime; optional operator note. |
70+
| `public_base_url` | `SM_PUBLIC_BASE_URL` | `""` | Runtime; public origin for canonical / `og:url`. Never derived from request `Host`. |
71+
72+
`merge_host_settings()` reads stored values before constructing middleware and
73+
registries on `app.state.sm.settings`. Runtime hydration populates
74+
`app.state.host.settings` with the same source precedence. Editing a boot-only
75+
field in the DB takes effect after restart; runtime fields can change live
76+
unless env pins them. See [Settings](/framework/settings).
7677

7778
## Users module
7879

@@ -135,8 +136,9 @@ In production (`SM_ENVIRONMENT` not in `{development, testing}`), boot fails if
135136
The app loads `.env` via pydantic's `BaseSettings`. Order of precedence:
136137

137138
1. Actual environment variables.
138-
2. `.env` file in the current working directory.
139-
3. Defaults from the `Settings` class.
139+
2. The project `.env` discovered by walking up from the current directory (or under `SM_PROJECT_ROOT`).
140+
3. Stored DB overrides for host settings, when merged or hydrated.
141+
4. Defaults from the settings class.
140142

141143
Keep secrets out of `.env.example` — check in only non-secret defaults.
142144

‎framework/cli/simple_module_cli/templates/host/migrations/env.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
make_process_revision_directives,
2121
render_item,
2222
)
23+
from simple_module_hosting.migrations import ALEMBIC_DATABASE_URL_ATTRIBUTE
2324
from simple_module_hosting.settings import Settings
2425
from sqlalchemy import engine_from_config, pool
2526
from sqlalchemy.engine import make_url
@@ -51,8 +52,8 @@ def _get_url() -> str:
5152
(a stray app.db, nothing else) that printing the target, password masked,
5253
is worth the one line.
5354
"""
54-
settings = Settings()
55-
url = settings.database_url.replace("+aiosqlite", "").replace("+asyncpg", "+psycopg2")
55+
database_url = config.attributes.get(ALEMBIC_DATABASE_URL_ATTRIBUTE) or Settings().database_url
56+
url = database_url.replace("+aiosqlite", "").replace("+asyncpg", "+psycopg2")
5657
logger.info(
5758
"Migrating %s (cwd=%s)", make_url(url).render_as_string(hide_password=True), Path.cwd()
5859
)

‎framework/core/simple_module_core/leases.py‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,14 @@
2020
installs one with :func:`configure_leases` when the app is built and when a
2121
worker boots — Redis when ``SM_REDIS_URL`` is set, else the ``sm_lease`` table,
2222
else :class:`InProcessLeaseBackend`, which only coordinates one process.
23-
Backend errors propagate: a lease that cannot be confirmed is never reported
24-
as held.
23+
Acquire timeouts and connectivity failures log a warning and return False.
24+
Other backend errors propagate; an unconfirmed lease is never reported as held.
2525
"""
2626

2727
from __future__ import annotations
2828

2929
import asyncio
30+
import logging
3031
import random
3132
import threading
3233
import time
@@ -50,6 +51,10 @@
5051
# Seconds between attempts while waiting for a held lease.
5152
_POLL = 0.05
5253
_POLL_MAX = 0.5
54+
# Shortest deadline one acquire attempt gets, even when the wait has run out:
55+
# ``wait=0`` means "don't queue", not "fail a healthy backend that takes 60 ms".
56+
_ATTEMPT_FLOOR = 1.0
57+
logger = logging.getLogger(__name__)
5358

5459

5560
class LeaseNotAcquiredError(RuntimeError):
@@ -62,7 +67,8 @@ class LeaseBackend(Protocol):
6267
``acquire``/``semaphore_acquire`` return the new fence number, or ``None``
6368
when the lease (or every slot) is held by someone else. ``refresh`` and
6469
``release`` return ``False`` when *token* no longer holds it — expired,
65-
released, or taken by another owner.
70+
released, or taken by another owner. Connectivity failures during acquire
71+
should raise built-in ConnectionError; release may log and return False.
6672
"""
6773

6874
async def acquire(self, name: str, token: str, ttl: float) -> int | None: ...
@@ -216,7 +222,14 @@ async def acquire(self, *, blocking: bool | None = None, wait: float | None = No
216222
deadline = None if wait is None else time.monotonic() + wait
217223
delay = _POLL
218224
while True:
219-
fence = await self._try()
225+
remaining = (
226+
None if deadline is None else max(_ATTEMPT_FLOOR, deadline - time.monotonic())
227+
)
228+
try:
229+
fence = await asyncio.wait_for(self._try(), remaining)
230+
except (TimeoutError, ConnectionError):
231+
logger.warning("Lease %r acquire timed out or backend unavailable", self.name)
232+
return False
220233
if fence is not None:
221234
self.fence = fence
222235
return True

‎framework/hosting/simple_module_hosting/_lease_redis.py‎

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,21 @@
88
members whose score has passed, then admits while ``ZCARD < slots``.
99
* ``INCR <key>:fence`` hands out the fencing token, inside the same script.
1010
11-
Unlike the rate limiter there is no per-process fallback: a lease that Redis
12-
could not confirm must not be reported as held, so errors propagate.
11+
There is no per-process fallback: unavailable Redis cannot confirm a lease.
12+
Acquire connectivity errors become ConnectionError for the lease handle;
13+
release connectivity errors are logged and return False. Refresh errors propagate.
1314
"""
1415

1516
from __future__ import annotations
1617

1718
import asyncio
19+
import logging
1820
import os
1921
from typing import Any
2022

23+
logger = logging.getLogger(__name__)
24+
_SOCKET_TIMEOUT = 2.0
25+
2126
# Holder and fence-counter keys live under disjoint prefixes: a name is never
2227
# parsed, so ``foo:fence`` cannot collide with the counter of ``foo``.
2328
_HOLDER_PREFIX = "sm:lease:h:"
@@ -113,13 +118,33 @@ def _redis(self) -> Any:
113118
self._client = self._factory()
114119
else:
115120
import redis.asyncio as aioredis
116-
117-
self._client = aioredis.from_url(self._url)
121+
from redis.asyncio.retry import Retry
122+
from redis.backoff import NoBackoff
123+
124+
self._client = aioredis.from_url(
125+
self._url,
126+
socket_timeout=_SOCKET_TIMEOUT,
127+
socket_connect_timeout=_SOCKET_TIMEOUT,
128+
# One reconnect, no backoff: a stale pooled connection gets a fresh
129+
# socket, while the 2 s timeouts keep the worst case bounded.
130+
retry=Retry(NoBackoff(), 1),
131+
)
118132
self._owner = owner
119133
return self._client
120134

121135
async def _eval(self, script: str, keys: list[str], *args: Any) -> int:
122-
return int(await self._redis().eval(script, len(keys), *keys, *args))
136+
from redis.exceptions import ConnectionError as RedisConnectionError
137+
from redis.exceptions import TimeoutError as RedisTimeoutError
138+
139+
try:
140+
return int(await self._redis().eval(script, len(keys), *keys, *args))
141+
except (RedisConnectionError, RedisTimeoutError) as exc:
142+
if script in (_ACQUIRE, _SEM_ACQUIRE):
143+
raise ConnectionError("Redis lease acquire unavailable") from exc
144+
if script not in (_RELEASE, _SEM_RELEASE):
145+
raise
146+
logger.warning("Redis lease release unavailable (%s)", type(exc).__name__)
147+
return 0
123148

124149
@staticmethod
125150
def _lease_keys(name: str) -> list[str]:

‎framework/hosting/simple_module_hosting/_preapp_config.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,8 @@ def merge_host_settings(
228228
# relative sqlite paths against the discovered project root, and
229229
# reconstructing Settings from env alone would redo that resolution
230230
# against whatever the cwd happens to be.
231-
fixed = {"secret_key": secret_key, "database_url": bootstrap.database_url}
231+
fixed = bootstrap.model_dump(include=bootstrap.model_fields_set)
232+
fixed.update(secret_key=secret_key, database_url=bootstrap.database_url)
232233
# Lenient, like the lifespan hydration: a stored value that parses but no
233234
# longer validates (a Literal narrowed in an upgrade) falls back to its
234235
# default with a WARNING instead of taking down every worker — and with

‎framework/hosting/simple_module_hosting/host_settings.py‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,12 @@
1313

1414
from __future__ import annotations
1515

16-
from typing import Literal
16+
from typing import Any, Literal
1717

1818
from pydantic import Field, field_validator, model_validator
19-
from pydantic_settings import BaseSettings, SettingsConfigDict
19+
from pydantic_settings import BaseSettings, DotEnvSettingsSource, SettingsConfigDict
2020
from simple_module_core.discovery import DEFAULT_AUTH_PROVIDER
21+
from simple_module_core.dotenv import find_env_file
2122

2223
from simple_module_hosting import _host_settings_support as v
2324
from simple_module_hosting._api_version import default_api_version as _default_api_version
@@ -43,6 +44,16 @@ class HostSettings(BaseSettings):
4344
# that has nothing to do with this app.
4445
model_config = SettingsConfigDict(env_prefix="SM_", extra="ignore")
4546

47+
def __init__(self, **values: Any) -> None:
48+
if "_env_file" not in values:
49+
values["_env_file"] = find_env_file()
50+
super().__init__(**values)
51+
52+
@classmethod
53+
def env_seeded_fields(cls) -> set[str]:
54+
"""Let hydration rank discovered dotenv values above DB overrides too."""
55+
return set(DotEnvSettingsSource(cls, env_file=find_env_file())())
56+
4657
multi_tenant: bool = Field(default=False, json_schema_extra=_RESTART_TENANCY)
4758
tenant_header: str = Field(default="", json_schema_extra=_RESTART_TENANCY)
4859
default_tenant: str = Field(default="", json_schema_extra=_RESTART_TENANCY)

0 commit comments

Comments
 (0)