fix(ci): speed up snapshot downloading on CI#6592
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds FOREST_AUTO_DOWNLOAD_SNAPSHOT_PATH and wires it through CI, test harness scripts, and daemon startup so the auto-download snapshot path can be overridden when preparing or starting a Forest node. Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI Workflow
participant Harness as Test Harness
participant Downloader as aria2c (Downloader)
participant Daemon as Forest Daemon
CI->>Harness: set FOREST_AUTO_DOWNLOAD_SNAPSHOT_PATH
Harness->>Harness: handle_auto_download_snapshot_env()
alt env var set
Harness->>Downloader: download snapshot to specified path
Downloader-->>Harness: snapshot file saved
end
Harness->>Daemon: start node (with --auto-download-snapshot / path)
Daemon->>Daemon: maybe_set_snapshot_path reads env var
alt env var present
Daemon-->>Daemon: use provided snapshot path
else
Daemon-->>Daemon: resolve default snapshot URL
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Comment |
hanabi1224
requested review from
LesnyRumcajs
and removed request for
a team
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/forest.yml (1)
28-39:⚠️ Potential issue | 🔴 CriticalConfirmed CI regression:
calibnet_db_migration.shuses--auto-download-snapshotwithout pre-downloadThe global
FOREST_AUTO_DOWNLOAD_SNAPSHOT_PATHoverride will causecalibnet_db_migration.shto fail. This script runs--auto-download-snapshotin a Docker container without callinghandle_auto_download_snapshot_envfirst, meaning it expects the snapshot file at the configured path but never pre-downloads it.Other test scripts (
calibnet_kademlia_check.sh,calibnet_no_discovery_check.sh,calibnet_stateless_mode_check.sh, andharness.sh) properly callhandle_auto_download_snapshot_envbefore using the flag. Either scopeFOREST_AUTO_DOWNLOAD_SNAPSHOT_PATHto specific jobs that pre-download, or add the pre-download step tocalibnet_db_migration.sh.
🤖 Fix all issues with AI agents
In `@scripts/tests/harness.sh`:
- Around line 19-23: In handle_auto_download_snapshot_env replace the current
existence check with a non-empty check (e.g. [ -n
"${FOREST_AUTO_DOWNLOAD_SNAPSHOT_PATH:-}" ]), compute and quote the target
directory and filename into variables (dir="$(dirname
"${FOREST_AUTO_DOWNLOAD_SNAPSHOT_PATH}")" and file="$(basename
"${FOREST_AUTO_DOWNLOAD_SNAPSHOT_PATH}")"), create the directory if missing
(mkdir -p "$dir"), and call aria2c with quoted variables (aria2c -x5 -c
https://forest-archive.chainsafe.dev/latest/calibnet/ -d "$dir" -o "$file") so
empty values are guarded against and paths are properly handled in
handle_auto_download_snapshot_env.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@scripts/tests/harness.sh`:
- Around line 27-28: There is an extra closing brace after the function
definition (a stray `}` following the function closed at line 27) that causes a
bash syntax error; remove the redundant `}` so the function ends with its single
matching `}` (ensure the function's closing brace is the only one present) and
re-run the script to confirm it sources without syntax errors.
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
2 participants
Summary of changes
This PR tries to mitigate #3715 on CI
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Outside contributions
Summary by CodeRabbit
New Features
Documentation