fix: replace models.UUIDField with Char32UUIDField for MariaDB compatibility - #3008
fix: replace models.UUIDField with Char32UUIDField for MariaDB compatibility#3008xogas wants to merge 7 commits into
Conversation
ZUP779
left a comment
There was a problem hiding this comment.
PR #3008 Code Review 汇总报告
由 codex (gpt-5.5) + Claude Opus 4.7 [1m] 双模型 review,主 agent 汇总整理。
Report
Critical Issues
1. Char32UUIDField.get_db_prep_value() 在非原生 UUID 后端会抛 AttributeError,导致写入/查询全部失败
位置:
apiserver/paasng/paas_wl/utils/models.py:53-57apiserver/paasng/paasng/utils/models.py:233-237
当前实现:
def get_db_prep_value(self, value, connection, prepared=False):
value = super().get_db_prep_value(value, connection, prepared)
if value is not None:
value = value.hex
return value问题分析:Django 5.x UUIDField.get_db_prep_value() 的返回值取决于连接后端:
- 当
connection.features.has_native_uuid_field=True(MariaDB 10.7+、PostgreSQL):返回uuid.UUID对象,.hex可用 - 当
has_native_uuid_field=False(MySQL 非 MariaDB、MariaDB <10.7、SQLite 等):返回str(已是 32 位 hex 字符串),str没有.hex属性,会抛AttributeError
项目 settings 中使用 django.db.backends.mysql(见 paasng/settings/utils.py),兼容 MySQL 和 MariaDB。项目 settings __init__.py 中还有 MySQL 版本判断逻辑(mysql_version < (8, 0, 4)),说明确实需要兼容 MySQL 非 MariaDB 环境。在这些环境下,所有使用 Char32UUIDField 的字段写入都会崩溃。
[codex] 将此问题定为 Critical;[claude] 定为 High。主 agent 判定:由于项目明确兼容 MySQL(非 MariaDB),该问题会在非 MariaDB 环境下导致所有 UUID 字段写入直接崩溃,属于合并前必须修复的阻塞性问题,定为 Critical。
建议修复:
def get_db_prep_value(self, value, connection, prepared=False):
if value is None:
return None
if not isinstance(value, uuid.UUID):
value = self.to_python(value)
return value.hex或更保守地:
def get_db_prep_value(self, value, connection, prepared=False):
value = super().get_db_prep_value(value, connection, prepared)
if isinstance(value, uuid.UUID):
return value.hex
return value两处定义(paas_wl 和 paasng)必须同步修复。
High Issues
1. Char32UUIDField 在两处完全重复定义
位置:
apiserver/paasng/paas_wl/utils/models.py:41-57apiserver/paasng/paasng/utils/models.py:221-237
两个类的字段签名、docstring、db_type、get_db_prep_value 逐字符一致(经 diff 验证)。维护风险:
- Critical #1 的 bug 修复需要同步改两处,漏改任一处就是运行时错误
- 迁移文件中
paas_wl.utils.models.Char32UUIDField与paasng.utils.models.Char32UUIDField被混用(不同 app 的迁移引用不同路径),二者的deconstruct()绑定到不同类路径,未来若分裂实现会导致同库列语义不一致
[codex] 在 Dismissed Findings 中提及(不作为阻塞但指出需同步修复);[claude] 定为 High。主 agent 判定为 High:与 Critical #1 直接关联(重复定义导致修复必须双写),合并前应统一。
建议:在其中一处(推荐 paas_wl/utils/models.py)定义唯一 Char32UUIDField,另一处 re-export,保持向后兼容。
Medium Issues
1. 迁移文件包含与 UUID 兼容性无关的 DeleteModel 操作
位置:apiserver/paasng/paasng/misc/operations/migrations/0002_delete_applicationlatestop.py
DeleteModel(name='ApplicationLatestOp') 是数据破坏性 schema 操作,与 UUIDField 兼容性无关。如果私有部署仍依赖该表的历史数据,执行迁移后无法恢复。
[codex] 定为 Medium;[claude] 定为 Medium。主 agent 判定:Medium,建议拆分到单独 PR 或在 PR 描述中明确说明数据影响。
2. 迁移夹带非 UUIDField 的自动生成变更
位置:
apiserver/paasng/paasng/platform/templates/migrations/0013_alter_template_type.py:调整Template.type的 choicesapiserver/paasng/paasng/accessories/services/migrations/0010_alter_plan_uuid_and_more.py:PreCreatedInstance.binding_policy默认值迁移
这些是 makemigrations 顺带补齐的模型-迁移漂移,与核心目标关联弱。
[codex] 定为 Low;[claude] 定为 Medium(与 #1 合并描述)。主 agent 判定:Medium,建议在 PR 描述中逐项说明。
3. deployment.py 中残留 max_length=32 无意义参数
位置:apiserver/paasng/paasng/platform/engine/models/deployment.py:117-142
build_process_id = Char32UUIDField(max_length=32, null=True)
build_id = Char32UUIDField(max_length=32, null=True)
# ... 共 6 处UUIDField 从不使用 max_length(其 deconstruct 不会输出它),迁移文件中也确实缺省此参数。属于旧代码遗留的无意义参数,虽不致 bug,但造成 model 与迁移不一致,给后续读者传递错误信号。
[claude] 定为 Medium。[codex] 未单独提及。主 agent 判定:Medium,建议顺手清理。
4. 未新增针对 Char32UUIDField 的单元测试
Char32UUIDField 涉及关键行为:db_type 恒返回 char(32)、UUID 写入前去除连字符、读回时正常反序列化。尤其在 Critical #1 修复前后,需要覆盖"原生 UUID backend / 非原生 backend / None 值 / str 输入 / UUID 输入"的单元测试来防回归。当前 PR 无任何测试用例。
[claude] 定为 Medium。[codex] 未单独提及。主 agent 判定:Medium。
5. 主键 AlterField 大表在线迁移风险
生产环境核心表(Application.id、Module.id、EngineApp.id、Build.uuid、Deployment.id 等)已是 char(32)。理论上 ALTER TABLE ... MODIFY COLUMN 从 CHAR(32) 到 CHAR(32) 应为 metadata-only,但主键类型的 AlterField 在部分 MariaDB 版本仍可能触发表重建/长锁。建议在 PR 描述或迁移文件顶部加 runbook 提示。
[claude] 定为 Medium。[codex] 未单独提及。主 agent 判定:Medium,建议补充运维提示。
Low Issues
无
Dismissed Findings
- 旧 migration 文件中仍存在
models.UUIDField:历史迁移状态,当前 PR 已通过后续AlterField迁移把当前模型字段调整为Char32UUIDField,不应批量改写历史迁移。[codex] 驳回正确。 Char32UUIDField.db_type()对所有数据库都返回char(32):本 PR 的目标就是避免 MariaDB 10.7+ 使用原生 UUID 类型,统一使用char(32)与历史库表兼容。[codex] 驳回正确。if value is not None冗余:父类在value is None时返回None,若不加判断则None.hex会崩溃,此处非过度防御。[claude] 驳回正确。- 迁移文件时间戳为 2026-08-12:由 Django
makemigrations自动生成,与本机时间一致,非问题。[claude] 驳回正确。 - 字段命名
Char32UUIDField与 Django 官方建议不同:无强制规范,命名清晰即可。[claude] 驳回正确。 Char32UUIDField未 overridefrom_db_value:[claude] 提为 Low,当前父类UUIDField.from_db_value的str→UUID转换稳定工作,属于隐性依赖但非当前问题。主 agent 判定为非阻塞,归入 Dismissed。
Agent Agreement Summary
| Issue | Codex | Claude | Verdict |
|---|---|---|---|
get_db_prep_value 在非原生 UUID 后端对 str 调用 .hex 导致 AttributeError |
是 (Critical) | 是 (High) | Critical |
Char32UUIDField 在两处完全重复定义 |
提及 (Dismissed) | 是 (High) | High |
迁移包含无关的 DeleteModel(ApplicationLatestOp) |
是 (Medium) | 是 (Medium) | Medium |
| 迁移夹带非 UUIDField 自动生成变更(template type 等) | 是 (Low) | 是 (Medium) | Medium |
deployment.py 残留 max_length=32 无意义参数 |
否 | 是 (Medium) | Medium |
未新增 Char32UUIDField 单元测试 |
否 | 是 (Medium) | Medium |
主键 AlterField 大表在线迁移风险 |
否 | 是 (Medium) | Medium |
Char32UUIDField 未 override from_db_value 的脆弱性 |
否 | 是 (Low) | Dismissed(非阻塞) |
合并建议:merge after fixes(修复后合并)
理由:PR 的修复方向正确,将 UUIDField 替换为 Char32UUIDField 以兼容 MariaDB 10.7+ 是 Django 5.0 升级的必要操作,迁移方案整体合理。但 Critical #1(get_db_prep_value 在非原生 UUID 后端会抛 AttributeError)会影响 MySQL(非 MariaDB)环境下所有 UUID 字段的写入,属于合并前必须修复的阻塞性问题。High #1(重复定义)与 Critical #1 直接关联,应一并统一。Medium 问题中,无关迁移和缺少测试至少需要在 PR 描述中显式说明。修复上述问题后可以放心合并。
由 codex (gpt-5.5) + Claude Opus 4.7 [1m] 双模型 review,主 agent 汇总 | [from openclaw-internal]
|
C1 修复, 移除 与 UUID 兼容性无关 的迁移文件, 其他不做修改 |
|
|
后续约定:新建 UUID 字段请使用
|
|
第一个已经修改了, 哪个不止在 mysql 下失败, mariadb 感觉也会失败 第二个是因为原本就有两者在 paasng, paas_wl 中存在冗余的字段, 这里采用原有设计, 保持一致, 不做收敛 第三个, 将添加 |
| * For Python files, run `ruff format` to format after edits. | ||
| * In API tests, add docstrings and type hints for fixtures, especially setup fixtures. | ||
| * Preserve in-function guidance comments in test fixtures during refactors (for example setup/teardown hint comments). | ||
| * For UUID model fields, use `Char32UUIDField` instead of `models.UUIDField`. Reason: on Django 5.0+ with MariaDB 10.7+, `models.UUIDField` maps to native UUID instead of legacy `CHAR(32)`. Existing columns stay `CHAR(32)`, so writing dashed UUID values can fail with "Data too long". |
There was a problem hiding this comment.
可以缩减一下表达,agents 里面的东西占 context,可以尽量精简
感觉可以,甚至可以使用 django 的 system check 机制增加一个检查,禁止使用 models.UUIDField。已有的多租户字段和 perm_check 应该就是这么检查的。 |
|
已添加 sysytem check |
Refs: https://docs.djangoproject.com/en/5.2/releases/5.0/#migrating-existing-uuidfield-on-mariadb-10-7