Skip to content

record end_time in auto_rules_task_log - #909

Merged
AlexCXC merged 1 commit into
masterfrom
record-auto-rule-end-time
Sep 8, 2026
Merged

AlexCXC merged 1 commit into
masterfrom
record-auto-rule-end-time

Conversation

@AlexCXC

@AlexCXC AlexCXC commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@seafile-dev seafile-dev left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: Comment

Warnings:

  • 新增结束时间计算及 INSERT 参数缺少针对 manager 的回归测试;建议覆盖正常小数秒、run_time=0 与 SQL 参数绑定。详见行内评论。

'owner': auto_rule_result.owner,
'trigger_time': auto_rule_result.trigger_time,
'trigger_date': auto_rule_result.trigger_date,
'end_time': auto_rule_result.trigger_time + timedelta(seconds=auto_rule_result.run_time),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Warning] 新增时间计算缺少回归覆盖

Why this matters:
这里新增了 end_time 的计算并作为 INSERT 参数写库,但本 PR 没有覆盖 update_stats() 的测试。小数秒、资源限流产生的 run_time=0 或参数绑定回归都会在异步统计链路中静默影响记录。

Suggested fix: 为 AutomationsStatsManager.update_stats() 添加 mock session 单测,断言 end_time == trigger_time + timedelta(seconds=run_time),并覆盖小数秒、零耗时和 SQL 实际携带 :end_time

@seafile-dev seafile-dev left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: Request changes

Blocking:

  • 现有部署和全新安装的 auto_rules_task_log schema 均未新增 end_time;详见行内评论。

insert_rule_log = '''
INSERT INTO auto_rules_task_log (trigger_time, success, rule_id, run_condition, dtable_uuid, org_id, owner, warnings) VALUES
(:trigger_time, :success, :rule_id, :run_condition, :dtable_uuid, :org_id, :owner, :warnings)
INSERT INTO auto_rules_task_log (trigger_time, end_time, success, rule_id, run_condition, dtable_uuid, org_id, owner, warnings) VALUES

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] 缺少数据库升级脚本

Why this matters:
现有 SeaTable 部署的 auto_rules_task_log 尚无 end_time 列,而这里会在每次自动化完成时执行包含该列的 INSERT,导致 MySQL 报“Unknown column”,统计更新和日志写入都会回滚。配套 dtable-web PR 也没有更新初始建表 SQL,因此新安装同样不具备该列。

Suggested fix: 在 dtable-web 的对应版本升级 SQL 中增加可重复执行的 ALTER TABLE auto_rules_task_log ADD COLUMN ... end_time DATETIME(6) NULL,并同步更新 sql/mysql.sql 的建表定义;部署 dtable-events 前必须先执行该迁移。

@AlexCXC
AlexCXC merged commit 1338315 into master Sep 8, 2026
1 check passed
@AlexCXC
AlexCXC deleted the record-auto-rule-end-time branch September 8, 2026 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants