feat: 支持空间变量 --story=132692335#633
Conversation
# Reviewed, transaction id: 75794
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review 总结
本 PR 新增了 variable_manager 模块，实现空间级变量的 CRUD 接口，并将任务启动时的空间变量注入从 EngineSpaceConfig 迁移到新的变量服务。整体结构清晰，文档齐全，迁移文件也已同步。
以下几个问题需要关注：
⚠️ 逻辑问题 — start() 缺少错误处理，API 失败会直接抛出 KeyError
文件: bkflow/task/operations.py，新增行：
space_var = InterfaceModuleClient().get_variable(self.task_instance.space_id)
root_pipeline_context.update(space_var["data"])若 get_variable 接口请求失败或返回非预期结构（如无 "data" 字段），space_var["data"] 会直接抛出 KeyError，导致任务启动失败且无日志。建议加 .get("data", {}) 并在接口调用前后添加日志。
⚠️ 逻辑问题 — get_variable_list 使用 require_POST 语义不符
文件: bkflow/apigw/views/get_variable_list.py
查询接口使用了 @require_POST，但按 REST 规范查询应使用 GET（规范第 4 节）。若因兼容网关限制必须用 POST，建议在注释里说明原因。
⚠️ 逻辑问题 — validate_type 使用硬编码字符串，应抽为常量
文件: bkflow/variable_manager/serializers.py，第 57 行：
if value not in ["space", "scope"]:"space"、"scope" 是业务关键枚举值，应在 constants.py 中定义为 VARIABLE_TYPE_SPACE = "space" 等常量，与规范第 2 节一致。
⚠️ 逻辑问题 — create 方法中 Space 存在性校验应移到 validate 而非 create
文件: bkflow/variable_manager/serializers.py，第 63-65 行：
def create(self, validated_data):
...
if not Space.objects.filter(id=space_id).exists():
raise serializers.ValidationError(...)create() 是持久化方法，校验逻辑应在 validate() 或 validate_space_id() 中完成（规范第 1 节），否则 is_valid() 通过后调用 save() 才抛出校验异常，行为反直觉。
🔒 安全问题 — VariableInternalViewSet.get_variable 缺少 is_deleted 过滤
文件: bkflow/variable_manager/views.py，第 63 行：
variables = self.queryset.filter(space_id=space_id, type="space").only("key", "value")queryset = VariableManager.objects.all() 未过滤软删除数据，已删除的变量依然会被注入到任务上下文中，可能导致意外行为。应加 is_deleted=False 过滤条件。
⚠️ 逻辑问题 — delete_variable 未过滤软删除记录
文件: bkflow/apigw/views/delete_variable.py，第 40 行：
variable = VariableManager.objects.get(id=variable_id, space_id=space_id)查询时未加 is_deleted=False，对已软删除的变量再次调用 delete() 会重复执行物理删除（若模型 delete() 是硬删除）或逻辑混乱。应与 update_variable.py 保持一致，加上 is_deleted=False。
✨ 改进建议 — create_variable 缺少 result 字段在异常返回中
文件: bkflow/apigw/views/create_variable.py，第 52 行：
return {"code": err_code.ERROR.code, "message": f"创建变量失败: {str(e)}"}返回体缺少 "result": False，与同文件其他返回格式及项目约定不一致（update_variable.py 同样有此问题）。
# Reviewed, transaction id: 75807
There was a problem hiding this comment.
Code Review Summary
本次 PR 新增了变量管理模块（variable_manager），包含 CRUD 接口、API 网关接入和内部接口对接，总体结构清晰、文档齐全。发现以下几个需要关注的问题：
🚨 严重 /
1. delete_variable.py — 硬删除与软删除字段不一致
variable.delete() 执行的是 Django 硬删除，但模型已有 is_deleted 软删除字段，且查询条件也用 is_deleted=False 过滤。应改为 variable.is_deleted = True; variable.save() 或调用项目公共的软删除方法，否则硬删除后 is_deleted 过滤逻辑形同虚设。
2. task/operations.py — get_variable() 无错误处理
space_var = InterfaceModuleClient().get_variable(...) 之后直接 space_var["data"]，若接口调用失败（网络异常、返回非 200）result=False，此处会 KeyError 或抛出异常导致任务启动失败。应判断 space_var.get("result") 后再取 data。
3. variable_manager/views.py — VariableInternalViewSet 基础 queryset 包含已删除记录
queryset = VariableManager.objects.all() 未过滤 is_deleted=False，虽然 get_variable action 内部额外加了过滤，但 AdminModelViewSet 继承的其他默认 action（list/retrieve 等）可能暴露软删除数据。
4. variable_manager/views.py — space_id 直接 [] 取值无 KeyError 防护
space_id = request.query_params["space_id"] 若调用方未传参会抛出 KeyError，应使用 .get("space_id") 并做非空校验，或用 Serializer 校验 query params。
5. update_variable.py — 部分错误响应缺少 result 字段
json.JSONDecodeError 和最外层 except 的返回均缺少 "result": False，与 create_variable.py 的响应格式不一致，调用方无法统一判断。
6. api-resources.yml — delete_variable / update_variable 使用 POST
删除操作用 POST /delete_variable/，更新操作用 POST /update_variable/，不符合 REST 语义（规范要求删除用 DELETE，增量更新用 PATCH）。
✨ 改进
7. constants.py — VARIABLE_TYPE_SPACE 命名易混淆
该常量包含 space 和 scope 两个值，但名称 VARIABLE_TYPE_SPACE 暗示只有 space 类型，建议改为 VALID_VARIABLE_TYPES 或 VARIABLE_TYPE_CHOICES。
There was a problem hiding this comment.
Code Review 总结
本轮对比上一轮评审，大部分问题已得到修复，以下为复核结果：
✅ 已解决的问题
get_variable_listHTTP 方法：已从require_POST改为require_GET，REST 语义正确- 硬编码类型字符串：已抽取为
VariableType枚举和VARIABLE_TYPE_SPACE常量 - Space 存在性校验：已从
create()移至validate_space_id()，校验时机正确 get_variableactionis_deleted过滤：已在查询中加入is_deleted=Falsedelete_variable未过滤已删除记录：已加入is_deleted=False过滤
🚨 仍需修复的问题
1. operations.py — get_variable() 无错误处理（Critical）
space_var["data"] 直接访问：若接口调用失败（网络异常、result=False），会直接抛出 KeyError 导致任务启动中断且无日志。两处均受影响（start() 和 get_node_data()）。
# 建议改为：
if not space_var.get("result"):
logger.error(f"get space variable failed: {space_var.get('message')}")
space_var_data = {}
else:
space_var_data = space_var.get("data", {})
root_pipeline_context.update(space_var_data)2. delete_variable.py — 硬删除与软删除字段不一致（Important）
模型有 is_deleted 软删除字段，查询也用 is_deleted=False 过滤，但 variable.delete() 执行的是 Django 硬删除，二者语义冲突。已删除的变量不会留下软删除记录，is_deleted 过滤逻辑形同虚设。建议改为：
variable.is_deleted = True
variable.save(update_fields=["is_deleted"])3. views.py — request.query_params["space_id"] 无防护（Important）
VariableInternalViewSet.get_variable 中直接用 [] 取值，调用方未传参时会抛出 KeyError 而非返回 400。建议改为 .get("space_id") 并做非空校验。
4. update_variable.py — JSONDecodeError 响应缺少 result 字段（Minor）
第 52 行 json.JSONDecodeError 的返回体缺少 "result": False，与同文件及项目其他接口的响应格式不一致，调用方无法统一判断。
总体评价：本 PR 变量管理模块结构清晰，文档齐全，迁移文件已同步，本轮修复了多个校验和 REST 规范问题。上述 4 个遗留问题中，operations.py 的错误处理和 delete_variable 的删除语义问题对运行时稳定性影响较大，建议优先处理。
# Reviewed, transaction id: 75816
There was a problem hiding this comment.
Code Review 总结（本轮复核）
对照上一轮评审（commit b7a40001）与当前 HEAD（6a4c7b9）的变更进行复核。
✅ 上轮已确认解决的问题（本轮验证保持已解决）
get_variable_listHTTP 方法（POST → GET）- 硬编码类型字符串 →
VariableType枚举 +VARIABLE_TYPE_SPACE常量 - Space 存在性校验从
create()移至validate_space_id() get_variableaction 加入is_deleted=False过滤delete_variable加入is_deleted=False过滤delete_variable.delete()软删除语义：CommonModel.delete()已重写为软删除，✅ 无问题
🚨 仍未解决的问题
1. bkflow/task/operations.py 第 191 行 — space_var["data"] 无错误处理（Critical）
space_var = InterfaceModuleClient().get_variable(self.task_instance.space_id)
root_pipeline_context.update(space_var["data"]) # ← 若 result=False，直接 KeyError接口调用失败时（网络异常、服务不可用），space_var 中无 "data" 键，会直接抛出 KeyError 导致任务启动中断且无任何业务日志。建议：
if not space_var.get("result"):
logger.error("get space variable failed: %s", space_var.get("message"))
space_var_data = {}
else:
space_var_data = space_var.get("data", {})
root_pipeline_context.update(space_var_data)2. bkflow/variable_manager/views.py 第 62 行 — request.query_params["space_id"] 无防护（Important）
space_id = request.query_params["space_id"] # ← 未传参时抛 KeyError，返回 500调用方未传 space_id 时会返回 500 而非 400。建议改用 .get("space_id") 并做非空校验，或用 Serializer 校验 query params。
3. bkflow/apigw/views/update_variable.py 第 52 行 — JSONDecodeError 响应缺少 result 字段（Minor）
return {"code": err_code.VALIDATION_ERROR.code, "message": "请求体JSON格式错误"}
# ← 缺少 "result": False，与项目响应格式约定不一致总体评价：本 PR 变量管理模块结构清晰，文档齐全，迁移文件已同步，本轮又解决了多个遗留问题。operations.py 的错误处理问题对运行时稳定性影响最大，建议优先处理。
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #633 +/- ##
==========================================
Coverage ? 83.04%
==========================================
Files ? 262
Lines ? 14720
Branches ? 0
==========================================
Hits ? 12224
Misses ? 2496
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
# Reviewed, transaction id: 75817
There was a problem hiding this comment.
Code Review 总结（本轮复核 HEAD: cdca84c）
对照上一轮评审（commit 6a4c7b9）与当前 HEAD（cdca84ca）的变更进行复核。
✅ 上轮已解决的问题
start()中get_variable()增加了result判断与错误日志 ✅delete_variable软删除语义（CommonModel.delete()已重写）✅get_variable_list改为require_GET✅Space存在性校验已移至validate_space_id()✅get_variableaction 加入is_deleted=False过滤 ✅
🚨 仍未解决的问题
1. bkflow/task/operations.py — get_node_data() 中 space_var["data"] 无错误处理（Critical）
start() 已修复，但 get_node_data() 同样的调用仍直接 space_var["data"]，接口失败时会 KeyError 导致节点数据获取中断且无业务日志。
# 建议：
if not space_var.get("result"):
logger.error("get space variable failed: %s", space_var.get("message"))
space_var_data = {}
else:
space_var_data = space_var.get("data", {})
root_pipeline_context.update(
{key: {"type": "plain", "value": value} for key, value in space_var_data.items()}
)2. bkflow/variable_manager/views.py — request.query_params["space_id"] 无防护（Important）
调用方未传 space_id 时抛出 KeyError，返回 500 而非 400。请改用 .get("space_id") 并做非空校验，或用 Serializer 校验 query params。
3. bkflow/variable_manager/views.py — VariableInternalViewSet.queryset 含已删除记录（Important）
queryset = VariableManager.objects.all() 未过滤 is_deleted=False，AdminModelViewSet 继承的默认 list/retrieve 等 action 可能暴露已软删除数据。建议改为 VariableManager.objects.filter(is_deleted=False)。
4. bkflow/apigw/views/update_variable.py — JSONDecodeError 响应缺 result 字段（Minor）
第 52 行返回体 {"code": ..., "message": ...} 缺少 "result": False，与项目其他接口约定不一致，调用方无法统一判断。
总体评价：start() 的错误处理已完善，本轮 get_node_data() 存在同样未修复的问题，建议一并处理。VariableInternalViewSet 的 queryset 过滤和 space_id 参数防护问题对线上稳定性影响较大，请优先处理。
2 participants
Reviewed, transaction id: 75794