iomgaa
afd6101c08
Mutation testing showed the negative structured-retries and expected-dim checks in the env parsing path could be deleted with every test still passing. Their value is the env key name in the message, so they need tests that assert it. Changelog now states the real scope of this release and warns that normalising scope moves the Redis keys, the one change here that silently relocates runtime state. Records the breaker threshold derivation as deliberately env-only so it does not resurface as another round.
165 lines
12 KiB
Markdown
165 lines
12 KiB
Markdown
# GatewaySettings 装配校验补齐(第二轮)
|
|
|
|
- **日期**: 2026-07-30;**状态**: **已批准并实施**(2026-07-30 人类门通过;§9 结论、§10 实施留痕)
|
|
- **缘起**: [2026-07-29-settings-invariant-guards-design.md](2026-07-29-settings-invariant-guards-design.md) §9.1 —— 独立 verifier 在第一轮交付后发现,`from_env` 上还留着一批同族校验;本设计是那一轮的续作,**同一个 bug 类的剩余部分**
|
|
- **上游依据**: 第一轮设计 §2 已批准的方案 A(不变量归属于类,不归属于某个工厂);CLAUDE.md §4.3(assert 仅用于内部不变量)、§4.5(装配只有两条路)
|
|
|
|
## 1. 待收拢的校验清单(逐条实测确认只在 `from_env` 生效)
|
|
|
|
### A. 枚举合法域(6 条)
|
|
|
|
| 字段 | 合法域 | 现居 |
|
|
|---|---|---|
|
|
| `limiter_backend` / `breaker_backend` | `{memory, redis}` | `_load_pgw`(经 `_load_choice`) |
|
|
| `cache_backend` | `{redis, memory, none}` | `_load_pgw` 内联 |
|
|
| `telemetry_backend` | `{sqlite, postgres, none}` | `_load_pgw` 内联 |
|
|
| `selector` | `_SELECTORS` | `from_env` 调 `_load_choice` |
|
|
| `quota_full` | `_QUOTA_FULL` | `from_env` 调 `_load_choice` |
|
|
|
|
直接构造传 `selector="random"` 或 `cache_backend="rediss"` 一律放行,后果是装配时落进 `_build_*` 的 else 分支或静默不建后端。
|
|
|
|
### B. 条件必填(7 条,跨字段)
|
|
|
|
| 条件 | 要求 | 违反后果 |
|
|
|---|---|---|
|
|
| `limiter_backend`/`breaker_backend`/`cache_backend` 取 `redis` | `redis_url` 非空 | **见 §2**,最严重 |
|
|
| `cache_backend != "none"` | `cache_namespace` 非空 | 缓存 key 失去租户隔离——踩"无缓存毒化"铁律 |
|
|
| `cache_backend != "none"` | `cache_ttl_s > 0` | `from_env` 明令禁止的"永不过期"从另一条路进来 |
|
|
| `telemetry_backend == "sqlite"` | `telemetry_sqlite_path` 非空 | 断言炸或写空路径 |
|
|
| `telemetry_backend == "postgres"` | `telemetry_pg_dsn` 非空 | 同上 |
|
|
|
|
### C. 标量域(2 条)
|
|
|
|
`structured_max_retries ≥ 0`;`scope` 非空(空 scope 会污染遥测与缓存命名空间)。
|
|
|
|
## 2. 为什么这批比第一轮更严重:`client.py` 的断言前提为假
|
|
|
|
`client.py` 有 5 处断言**明文声称这个前提已经成立**:
|
|
|
|
```python
|
|
assert settings.redis_url is not None # 内部不变量: config 已校验
|
|
```
|
|
|
|
位置:`client.py:262/282/302`(redis_url)、`:312`(pg_dsn)、`:316`(sqlite_path)。走 `from_settings` 时该注释是假的,verifier 实测:
|
|
|
|
| 运行方式 | 结果 |
|
|
|---|---|
|
|
| 断言开启 | `AssertionError()` —— 裸断言,不点字段、不说原因 |
|
|
| `python -O` | 断言消失,退化为 redis 库的 `ValueError: Redis URL must specify one of the following schemes...` |
|
|
|
|
后者正是 CLAUDE.md §4.3 禁止的"assert 承担生产校验"。
|
|
|
|
**但注意结论的方向**:这 5 处 assert 本身不是要修的东西——它们要的前提是对的,错的是没人保证这个前提。§4 给出处置。
|
|
|
|
## 3. 方案
|
|
|
|
沿用第一轮已批准的方案 A,不重新论证:全部收进 `GatewaySettings.__post_init__`,新增三个私有方法与既有四个并列。
|
|
|
|
| 方法 | 覆盖 |
|
|
|---|---|
|
|
| `_validate_backends` | A 类 6 条枚举 + B 类 redis_url 三条件 |
|
|
| `_validate_cache` | `cache_namespace` 非空、`cache_ttl_s > 0`(仅 `cache_backend != "none"` 时) |
|
|
| `_validate_telemetry` | sqlite path / postgres dsn 条件必填 + §5 的 DSN 形态 |
|
|
|
|
标量两条(`structured_max_retries`、`scope`)并入 `_validate_sources` 改名后的 `_validate_identity`,与 `SourceConfig._validate_identity` 同名同职。
|
|
|
|
枚举合法域上提为模块级 frozenset 常量(`_LIMITER_BACKENDS` 等),`_load_pgw` 与 `__post_init__` 共用一份,消除现有的内联字面量重复。
|
|
|
|
**否决的替代**:在 `_build_limiter`/`_build_cache` 等工厂函数里逐个补显式检查。理由同第一轮 §2 方案 B——校验散落在消费点,每加一个后端就多一处要同步,且 `dataclasses.replace` 仍绕过。
|
|
|
|
## 4. 5 处 assert 的处置:**保留,不改**
|
|
|
|
修好构造期校验后,`settings.redis_url is not None` 就真的成了内部不变量——CLAUDE.md §4.3 原文"assert 仅用于内部不变量"说的正是这种用法,同时它给类型检查器收窄了 `str | None`。此时删掉 assert 反而丢失类型信息,改成 `raise` 则是在防御一个已被构造期排除的情况(死代码)。
|
|
|
|
**要改的是注释**:`# 内部不变量: config 已校验` 应点明由谁保证,例如 `# 内部不变量: GatewaySettings._validate_backends 已保证`。前一轮的教训就是这类注释会随时间变成谎言。
|
|
|
|
## 5. Postgres DSN:校验而非规范化(本轮唯一的新决策)
|
|
|
|
`_load_pg_dsn` 对 `from_env` 读到的 DSN 做了**规范化**:剥掉 SQLAlchemy 风格的 `+asyncpg` 驱动后缀(asyncpg 不认)。直接构造那条路不会剥,`postgresql+asyncpg://...` 会原样送进 asyncpg 然后在首次写遥测时才炸。
|
|
|
|
| 选项 | 权衡 |
|
|
|---|---|
|
|
| A. 构造期校验,含 `+driver` 即报错 | 显式,库不碰用户给的值;但两条装配路对同一输入接受度不同 |
|
|
| B. 构造期静默剥后缀 | 两条路完全对齐;但 frozen 类在构造期悄悄改字段,调用方不知情 |
|
|
| **C. 构造期剥后缀 + `logger.warning`(用户 2026-07-30 拍板)** | 两条路行为对齐,同时不静默——调用方在日志里看得见库动了他的值,想根治就自己改 DSN |
|
|
|
|
选 C。实现要点:`object.__setattr__` 改 frozen 字段(`SourceConfig` 无此先例,但 frozen 的约束是对**外部**不可变,构造期规范化是既有 dataclass 惯用法);warning 走 loguru(核心依赖,库内 `ocr.py:183`/`embedding.py:318` 同款用法)。
|
|
|
|
**warning 不会打扰 env 用户**:`_load_pg_dsn` 保留现有的剥离逻辑,`from_env` 传给构造函数时 DSN 已经干净,`__post_init__` 无事可做。只有手工构造传了带后缀的 DSN 才会触发。三项目 `.env` 里那些 SQLAlchemy 写法不会每次装配刷一条 warning。
|
|
|
|
代价是同一件事有两处剥离逻辑。用同一个模块级 helper `_strip_dsn_driver(dsn)` 供两处调用,避免实现分叉。
|
|
|
|
## 6. 行为审计
|
|
|
|
| 现有行为 | 处置 |
|
|
|---|---|
|
|
| `from_env` 对上述 15 条的校验与报错 | **全部保留**,时机提前到 `cls(...)`;`_load_*` 内联检查删除,避免同一约束两处维护 |
|
|
| `_load_pg_dsn` 剥 `+driver` | **保留**,继续只在 env 路径生效(§5) |
|
|
| `_load_choice` 的 `default` 语义(键缺失时取默认) | **保留**,那是 env 解析职责,不是不变量 |
|
|
| `_load_breaker` 的有效阈值派生 `max(配置值, 源级并发×2)` | **有意保留在 env 层**(verifier 二次核验点名,记此备案免成"第五批")。它是**派生**不是校验/规范化:两路产出确实不同(env 装配 threshold=5/并发=100 得 200,直接构造得 5),但派生依赖的是"用户没显式表态时库替他选一个合理值"的 env 语义;代码构造那条路,调用方给什么就是什么表态。其跨字段下限风险由 `_validate_probe` 在构造期兜底 |
|
|
| 直接构造出上述任一非法组合 → 静默成功 | **有意替换**为构造期 `ValueError` |
|
|
| `client.py` 5 处 assert | **保留**,仅改注释(§4) |
|
|
| 异常类型 | 一律 `ValueError`,与第一轮及既有装配错误一致 |
|
|
|
|
**有意放弃**:不校验 `pricing_path` 指向的文件是否存在(I/O 不属于配置校验,`PricingTable.from_file` 自会报错);不强制 `cache_backend == "none"` 时 namespace/ttl 必须为 None(多余字段无害)。
|
|
|
|
## 7. 非功能维度
|
|
|
|
与第一轮同构,不重复论证:`__post_init__` 纯同步计算无 I/O(不适用并发/取消/持久化);装配期属准入侧,报错不放行;`__post_init__` 不改字段故幂等。**性能**:新增约 10 次字符串比较,第一轮实测单次构造 1.45 µs 且库内无热路径构造 `GatewaySettings`,可忽略。
|
|
|
|
## 8. 测试策略
|
|
|
|
`tests/unit/test_config.py::TestCrossFieldInvariants` 扩充(不新建类,同族不变量归一处):
|
|
|
|
| 用例组 | 断言 |
|
|
|---|---|
|
|
| 6 条枚举各一条非法值 | 抛 `ValueError`,消息含字段名与合法域 |
|
|
| redis_url 三条件(limiter/breaker/cache 各一) | 抛 `ValueError`,消息点明需要 `redis_url` |
|
|
| cache namespace 缺失 / ttl ≤ 0 | 抛 `ValueError` |
|
|
| telemetry sqlite path / pg dsn 缺失 | 抛 `ValueError` |
|
|
| `structured_max_retries=-1`、`scope=""` | 抛 `ValueError` |
|
|
| pg dsn 含 `+asyncpg`(直接构造) | 后缀被剥,字段值为干净 DSN,且发出一条 warning(用 `caplog`/loguru sink 断言) |
|
|
| pg dsn 干净(直接构造)、或经 `from_env` 传入 | **不发** warning——env 路已在 `_load_pg_dsn` 剥过,不该刷噪音 |
|
|
| 合法组合(每种 backend 组合各一) | 构造成功——收紧的是错的那些 |
|
|
| **回归护栏**:`GatewayClient.from_settings` 走 redis 三后端的合法配置 | 装配成功,证明 assert 前提真的被保证了 |
|
|
|
|
TDD:先跑出红,预计 ≥14 条失败。要求同第一轮——每条实现改动都要有对应测试能杀死它。
|
|
|
|
版本:**1.0.2**(patch),CHANGELOG 同样单列"行为收紧"小节。
|
|
|
|
## 9. 人类拍板结论(2026-07-30)
|
|
|
|
| 问题 | 结论 |
|
|
|---|---|
|
|
| §5 DSN 处置 | **选 C**:构造期剥后缀 + `logger.warning`。不静默改用户的值,也不让两条装配路产出不一致 |
|
|
| §4 assert 处置 | **保留,只改注释**,点明由哪个方法保证前提 |
|
|
| 方案主体 | 沿用第一轮已批准的方案 A,无需重新论证 |
|
|
| 版本 | 1.0.2(patch) |
|
|
| **范围追加**(实施中经 verifier 发现后拍板) | G1-G4 四条同族遗漏一并纳入本轮;G1 的 scope 规范化取**静默**小写+strip(不告警——`from_env` 一直静默小写,scope 大小写不承载语义) |
|
|
|
|
## 10. 实施留痕
|
|
|
|
分支 `fix/settings-invariants-round-2`。TDD 两段:主体 15 条先 **16 failed**、G1-G4 追加 **9 failed**,实现后全绿(547 passed / 14 skipped,1.0.1 基线 516)。
|
|
|
|
### 10.1 独立 verifier 的关键发现
|
|
|
|
第一次核验判**有阻塞**,已修:
|
|
|
|
- **阻塞(本轮新引入)**:DSN 剥离的 warning 打印了完整连接串,**含明文密码**,而库内此前从无任何地方打印连接串——违反 P5。已改为只报 scheme 段变化,并补回归测试断言密码与 host/path 不进日志。
|
|
- **变异测试 27/28 被杀**,唯一存活的是 `_load_pgw` 里 `PGW_CACHE_BACKEND` 域检查删掉后仍全绿(该 env 层 raise 零覆盖)。已补 `test_cache_backend_whitelist`,与既有 `test_telemetry_backend_whitelist` 对称。
|
|
- **assert 处置经独立核验成立**:遍历所有可达构造路径均无法制造 assert 失败,`python -O` 下同样在构造期被拦(旧病症消失);唯一能触发的是 `object.__new__` 绕过 `__post_init__` 的人造路径,非公共 API。
|
|
- **frozen 语义无副作用**:`object.__setattr__` 后 `hash`/相等性/集合去重正常,`replace` 幂等不重复告警,`pickle`/`deepcopy` 不触发 `__post_init__` 故不重复告警,对外仍抛 `FrozenInstanceError`。
|
|
|
|
### 10.2 G1-G4:第三批遗漏(已纳入本轮)
|
|
|
|
verifier 通读 `_load_*` 后发现,除设计 §1 的 15 条外还有四条**规范化**只在 env 路生效——与本轮所修的 DSN 是同一类:
|
|
|
|
| | 内容 | 危害 |
|
|
|---|---|---|
|
|
| G1 | `scope` 小写化 | **最严重**:scope 进 Redis key,大小写不一致使限流/熔断状态分裂到两套命名空间,分布式治理静默失效 |
|
|
| G2 | `redis_url` 空串归 None | 空串骗过 `is None`,退化为 redis 客户端的连接串天书报错——正是本轮 CHANGELOG 声称已消除的那种 |
|
|
| G3 | `pricing_path` 空串归 None | 退化为 `Is a directory: '.'` |
|
|
| G4 | `EmbeddingSettings.batch_size`/`expected_dim` 域 | 该类无 `__post_init__`;晚一步到 client 构造才 fail-loud |
|
|
|
|
统一收进新增的 `GatewaySettings._normalize()`(在全部 `_validate_*` 之前跑)与 `EmbeddingSettings.__post_init__`。DSN 后缀因需看 backend 且需告警,规范化留在 `_validate_telemetry`。
|