feat: 添加 full diff 全量校验 #659
Conversation
| @@ -28,7 +29,16 @@ export class CreateSyncBinaryTask { | |||
| // 默认只同步 binaryName 的二进制，即使有不一致的 category，会在同名的 binaryName 任务中同步 | |||
| // 例如 canvas 只同步 binaryName 为 canvas 的二进制，不同步 category 为 node-canvas-prebuilt 的二进制 | |||
| // node-canvas-prebuilt 的二进制会在 node-canvas-prebuilt 的任务中同步 | |||
| await this.binarySyncerService.createTask(binaryName as BinaryName); | |||
| await this.binarySyncerService.createTask(binaryName as BinaryName, { | |||
| fullDiff: isBetween2and205AM(), | |||
There was a problem hiding this comment.
我这边跑下来的数据，发现差异的有
bun 18
tfjs-models 25
nydus 16
sass-embedded 155
chromium-browser-snapshots 35
node 34
node-nightly 12
python 6
There was a problem hiding this comment.
另外 prisma 和 chrome-for-testing 确实不能fulldiff，除了这两个，还有其他不能跑的吗？跑fulldiff对我们这边的服务器压力倒不是很大
There was a problem hiding this comment.
我重新整理了一下，应该有四个adapter不能full diff，ChromeForTestingBinary，PlaywrightBinary，PrismaBinary，PuppeteerBinary。
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #659 +/- ##
=======================================
Coverage 96.82% 96.83%
=======================================
Files 180 180
Lines 17626 17651 +25
Branches 2293 2296 +3
=======================================
+ Hits 17067 17092 +25
Misses 559 559 ☔ View full report in Codecov by Sentry. |
WalkthroughThis update introduces enhancements to the binary synchronization feature. A new interface, Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (5)
- app/core/entity/Task.ts (2 hunks)
- app/core/service/BinarySyncerService.ts (5 hunks)
- app/port/schedule/CreateSyncBinaryTask.ts (2 hunks)
- config/binaries.ts (4 hunks)
- test/core/service/BinarySyncerService/executeTask.test.ts (1 hunks)
Additional comments not posted (9)
app/port/schedule/CreateSyncBinaryTask.ts (2)
38-43: Consider handling time zones inisBetween2and205AM.The current implementation does not account for time zones, which could lead to unexpected behavior depending on the server's time zone. Consider using
dayjswith a time zone plugin to ensure the check works correctly across different time zones.
32-33: EnsureenableFullDiffis correctly configured for each binary.Verify that the
enableFullDiffproperty is correctly set in the binary configurations to ensure that full diff tasks are created as intended.app/core/entity/Task.ts (2)
53-55: LGTM!The
CreateSyncBinaryTaskDatainterface correctly extendsTaskBaseDatawith an optionalfullDifffield.
80-80: LGTM!The
CreateSyncBinaryTasktype is correctly defined, utilizingCreateSyncBinaryTaskData.app/core/service/BinarySyncerService.ts (2)
115-117: Refine the logging message for clarity.Consider refining the logging message to include more context about the full diff operation, such as the binary being processed. This will improve the informativeness of the logs.
278-291: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [161-474]
Verify the performance impact of full diff logic.
Ensure that the full diff logic in
syncDirdoes not introduce significant performance overhead, especially for large directories. Consider optimizing the comparison logic if necessary.test/core/service/BinarySyncerService/executeTask.test.ts (1)
392-475: Add more scenarios to the full diff test case.The test case for the full diff operation is well-implemented. Consider adding more scenarios to cover edge cases, such as handling empty directories or directories with a large number of files, to ensure comprehensive testing.
config/binaries.ts (2)
26-26: The addition of theenableFullDiffproperty to theBinaryTaskConfigtype is correctly implemented and aligns with the PR objectives.
45-45: TheenableFullDiffproperty is correctly applied to thenode,chromium-browser-snapshots, andbunconfigurations. Consider reviewing other binary configurations to identify any additional candidates that could benefit from enabling the full diff check.Also applies to: 112-112, 916-916
#657 在diff方法中添加full diff，在每日凌晨两点创建full diff任务，全量校验
Summary by CodeRabbit