<feature>[longjob]: Introduce long job state and progerss notification#3293
Closed
MatheMatrix wants to merge 1 commit into5.5.6from
Closed
<feature>[longjob]: Introduce long job state and progerss notification#3293MatheMatrix wants to merge 1 commit into5.5.6from
MatheMatrix wants to merge 1 commit into5.5.6from
Conversation
GlobalConfigImpact Resolves: ZSTAC-81237 Change-Id: I7a6f6273626e766778686d756a737771636f616d Signed-off-by: AlanJager <ye.zou@zstack.io>
概览此次变更引入了进度更新的扩展点机制，使得进度持久化后能够触发下游通知。新增接口、通知消息类和SNS任务跟踪器，支持长任务的状态变化和进度更新的实时通知，并通过全局配置控制通知行为。 变更内容
时序图sequenceDiagram
participant CreateProgress as ProgressReportService
participant Registry as PluginRegistry
participant Extension as ProgressUpdateExtensionPoint<br/>Implementations
participant SNS as LongJobTaskTracker<br/>(SNS Publisher)
participant Database as DatabaseFacade
CreateProgress->>Database: persistProgress(vo)
Database-->>CreateProgress: ✓ Persisted
CreateProgress->>Registry: getPluginList(ProgressUpdateExtensionPoint)
Registry-->>CreateProgress: [Extension1, Extension2, ...]
loop For Each Extension
CreateProgress->>Extension: afterProgressPersisted(vo)
Note over Extension: Process notification
Extension->>SNS: publish task event
SNS-->>Extension: ✓ Published
Extension-->>CreateProgress: ✓ Complete
end
CreateProgress-->>CreateProgress: Persistence + Notification Cycle Complete
代码审查工作量估计🎯 3 (中等复杂度) | ⏱️ ~20 分钟 庆祝诗
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning, 1 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@header/src/main/java/org/zstack/header/longjob/LongJobProgressNotificationMessage.java`:
- Around line 79-92: In LongJobProgressNotificationMessage.progressUpdated, the
TaskProgressInventory content is set directly causing i18n mismatch; change it
to mirror ProgressReportService.inventory() by calling
toI18nString(progressVO.getContent()) when setting inv.setContent(...) and only
set inv.setArguments(...) when progressVO.getArguments() is non-null (and
likewise apply toI18nString to those arguments if ProgressReportService does so)
to ensure webhook notifications match API i18n formatting.
🧹 Nitpick comments (2)
longjob/src/main/java/org/zstack/longjob/LongJobGlobalConfig.java (1)
17-18: 建议补充全局配置校验，避免非法值进入运行时
新增配置缺少类型/范围校验，若被误设为非布尔或负值，可能导致通知逻辑异常。建议补充@GlobalConfigValidation或显式取值范围校验/默认值。core/src/main/java/org/zstack/core/progress/ProgressReportService.java (1)
414-422: 异常日志建议补充进度上下文
当前仅有固定文案，排查失败来源困难；建议带上 apiId/taskUuid/type 等关键信息。🔧 建议修改
- } catch (Throwable t) { - logger.warn("ProgressUpdateExtensionPoint.afterProgressPersisted failed", t); + } catch (Throwable t) { + logger.warn(String.format( + "ProgressUpdateExtensionPoint.afterProgressPersisted failed (apiId=%s, taskUuid=%s, type=%s)", + vo.getApiId(), vo.getTaskUuid(), vo.getType() + ), t); }
Comment on lines
+79
to
+92
| public static LongJobProgressNotificationMessage progressUpdated(LongJobVO vo, TaskProgressVO progressVO) { | ||
| LongJobProgressNotificationMessage msg = new LongJobProgressNotificationMessage(); | ||
| msg.longJob = LongJobInventory.valueOf(vo); | ||
| int percent = parseProgressContent(progressVO.getContent()); | ||
| msg.progress = percent; | ||
| TaskProgressInventory inv = new TaskProgressInventory(progressVO); | ||
| if (progressVO.getContent() != null) { | ||
| inv.setContent(progressVO.getContent()); | ||
| } | ||
| msg.taskProgress = inv; | ||
| msg.eventType = EventType.PROGRESS_UPDATED; | ||
| msg.timestamp = System.currentTimeMillis(); | ||
| return msg; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 定位 TaskProgressInventory 类文件
fd 'TaskProgressInventory\.java' -type fRepository: MatheMatrix/zstack
Length of output: 232
🏁 Script executed:
# 查找 ProgressReportService 相关文件
fd 'ProgressReportService\.java' -type fRepository: MatheMatrix/zstack
Length of output: 232
🏁 Script executed:
# 同时查找是否存在其他与任务进度库存相关的处理逻辑
rg 'class TaskProgressInventory' -A 30Repository: MatheMatrix/zstack
Length of output: 6153
🏁 Script executed:
# 查找 ProgressReportService 相关文件
fd 'ProgressReportService' -type fRepository: MatheMatrix/zstack
Length of output: 232
🏁 Script executed:
# 搜索 ProgressReportService 及其对 TaskProgressInventory 的处理
rg 'ProgressReportService' -A 5 -B 5Repository: MatheMatrix/zstack
Length of output: 50374
🏁 Script executed:
# 查看完整的 TaskProgressInventory 构造器实现和 content 相关方法
cat header/src/main/java/org/zstack/header/core/progress/TaskProgressInventory.javaRepository: MatheMatrix/zstack
Length of output: 2414
🏁 Script executed:
# 定位 ProgressReportService 的完整实现
find . -name "ProgressReportService.java" -path "*/core/progress/*"Repository: MatheMatrix/zstack
Length of output: 134
🏁 Script executed:
# 查看 ProgressReportService 中关于 TaskProgressInventory 的处理
rg 'TaskProgressInventory' core/src/main/java/org/zstack/core/progress/ProgressReportService.java -A 5 -B 5Repository: MatheMatrix/zstack
Length of output: 2551
🏁 Script executed:
# 查看 ProgressReportService 中关于 arguments 或 i18n 的处理
rg 'arguments|i18n' core/src/main/java/org/zstack/core/progress/ProgressReportService.java -A 3 -B 3Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
# 查看 toI18nString 方法的实现
rg 'toI18nString' core/src/main/java/org/zstack/core/progress/ProgressReportService.java -B 2 -A 10Repository: MatheMatrix/zstack
Length of output: 1070
🏁 Script executed:
# 查看 LongJobProgressNotificationMessage 的用途，是否用于 Webhook
rg 'LongJobProgressNotificationMessage' header/src/main/java/org/zstack/header/longjob/ -A 5 -B 2Repository: MatheMatrix/zstack
Length of output: 3339
应用 i18n 转换以保持任务进度内容一致
ProgressReportService.inventory() 方法在设置 content 时应用了 toI18nString() 转换，但此处直接设置 content 未进行相同处理。这会导致 Webhook 通知中的任务进度文案与 API 返回的文案不一致。建议参考 ProgressReportService.inventory() 的实现，对 content 应用 toI18nString() 转换：
inv.setContent(toI18nString(progressVO.getContent()));同时，arguments 的处理也应与 ProgressReportService 保持一致，仅在非空时设置。
🤖 Prompt for AI Agents
In
`@header/src/main/java/org/zstack/header/longjob/LongJobProgressNotificationMessage.java`
around lines 79 - 92, In LongJobProgressNotificationMessage.progressUpdated, the
TaskProgressInventory content is set directly causing i18n mismatch; change it
to mirror ProgressReportService.inventory() by calling
toI18nString(progressVO.getContent()) when setting inv.setContent(...) and only
set inv.setArguments(...) when progressVO.getArguments() is non-null (and
likewise apply toI18nString to those arguments if ProgressReportService does so)
to ensure webhook notifications match API i18n formatting.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GlobalConfigImpact
Resolves: ZSTAC-81237
Change-Id: I7a6f6273626e766778686d756a737771636f616d
Signed-off-by: AlanJager ye.zou@zstack.io
sync from gitlab !9114