Repository navigation
build(ci): rebuild the committed asset tables from pinned downloads in Rust - #403
Merged
Merged
Conversation
og dev build cn-intervals, callability and sv-rule-regions read the pinned downloads from a folder and write bin/cn_loci.tsv's interval rows, assets/callability_genes.tsv and assets/sv_rule_regions.tsv, byte for byte as the Python builders did. The GTF is read as bytes with a hand scanner of its attributes, the cut sets are merged once per chromosome and searched by binary search, and og never downloads. The SV rule builder now keeps the first GENCODE record per HGNC id and skips _PAR_Y copies, as the callability builder does; the committed table does not change. Bead: pgp-bs9u.35
build-callability.sh and build-sv-rule-regions.sh keep their downloads and hand the folder to og dev build; their Python heredocs are gone. build-cn-intervals.py becomes build-cn-intervals.sh with the same --list, --cache and --check. The three share scripts/ci/lib/fetch-pinned.sh: a cached file with the right sha256 is read in place, a 403 or 404 fails at once, anything else is retried for about five minutes. The marker line of bin/cn_loci.tsv names the new script; no row changes. The cn-intervals case, the e2e data fill and the converter checks of tests/test_copy_number.py call the new script and og. Bead: pgp-bs9u.35
Red-first control, reverted in the next commit: callability keeps the _PAR_Y copies, the SV rule builder keeps the last record per HGNC id, and each cn-intervals piece ends one base short. Bead: pgp-bs9u.35
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 44 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
…go red" The plant turned five builder tests red in Guard's rust job (run 43417543846); back to the right rules. Bead: pgp-bs9u.35
… failed fetch_pinned downloads to a mktemp file beside the target, so two runs on one cache never write the same partial file, and gives curl a 30 s connect timeout and a 1 KB/s for 60 s floor, so a stalled server reaches the retry or the error instead of holding the build. Bead: pgp-bs9u.35
Collaborator
Author
|
The review findings are addressed in 0948aae. The approved scope now covers all 26 changed paths. The PR body lists each path and its role in the port. The shared fetch loop checks every HTTP response. A 403 or 404 stops immediately. Other errors get at most six retries within the existing five-minute window. Connection and stall limits, checksum verification, cache hits and temporary-file cleanup remain covered. Red first: the new Rust tests ran against the previous fetch implementation at cebf6b9, using real HTTP responses on loopback: After the fix, all four fetch tests pass. Rust 1.93 runs 52 og-dev unit tests and nine repository tests successfully. Clippy with warnings denied, rustfmt and cargo doc also pass. The cache shell suite passes 30 checks. |
# Conflicts: # CHANGELOG.md # rust/crates/og-dev/src/lib.rs # rust/og/src/main.rs
…list zram0 Some runner images list /dev/zram0 as a disk of 0 bytes with no file system. The spare-disk step took it for the blank disk, and mkfs.ext4 stopped the leg before any case ran. The step now skips zram, ram and loop devices and any disk of 0 bytes.
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
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.
Problem
Three committed tables are rebuilt in CI from pinned public downloads: the interval rows of
bin/cn_loci.tsv,assets/callability_genes.tsvandassets/sv_rule_regions.tsv. Each builder was Python:scripts/ci/build-cn-intervals.pyand a heredoc insidescripts/ci/build-callability.shandscripts/ci/build-sv-rule-regions.sh. They had three download routines with two different retry rules. The SV rule builder kept the last GENCODE record of a gene, while the callability builder kept the first and skipped the_PAR_Ycopies.build-cn-intervals.py --checksaid "0 line(s) differ" for a catalog with an added line. The SV builder held 217 MB to write a 250 KB table.Fix
Three new verbs in the
ogbinary, inrust/crates/og-dev/src/build/:og dev build cn-intervalsscripts/ci/build-cn-intervals.py(deleted)og dev build callabilitybuild-callability.shog dev build sv-rule-regionsbuild-sv-rule-regions.shognever downloads. The shell scripts fetch the pinned files withcurlthrough one shared function,scripts/ci/lib/fetch-pinned.sh, then handogthe folder.build-cn-intervals.pyis nowbuild-cn-intervals.sh, with the same--list,--cacheand--check.The GTF is read as bytes, with a small scanner for its
key "value"pairs, only on the lines a builder keeps. The cut sets of the cn builder are merged once per chromosome and searched by binary search. Coverage usesog-intervalssets.What changes on purpose
bin/cn_loci.tsvnamesscripts/ci/build-cn-intervals.sh. No row changes. A marker naming another builder is still found.build-cn-intervals.sh --checkcounts the lines only in the file and the lines only in the rebuild, and says when only the order differs._PAR_Ycopies, as the callability builder does. No pseudogene of the pinned files is in a PAR, so the table does not change.Pipeline output names, formats, options and config keys stay unchanged. All table data rows stay byte for byte the same. The cn catalog marker names the new shell builder.
Measurements (MacMini-2, warm cache, the full script)
Files needed for the port
The approved scope includes these 26 paths. Each supports the implementation, caller switch or deletion.
rust/crates/og-dev/src/build/callability.rsrust/crates/og-dev/src/build/cn_intervals.rsrust/crates/og-dev/src/build/gtf.rsrust/crates/og-dev/src/build/mod.rsrust/crates/og-dev/src/build/sv_rules.rsrust/crates/og-dev/src/lib.rsrust/crates/og-dev/Cargo.tomlrust/Cargo.lockrust/og/src/main.rsscripts/ci/build-callability.shscripts/ci/build-cn-intervals.pyscripts/ci/build-cn-intervals.shscripts/ci/build-sv-rule-regions.shscripts/ci/lib/fetch-pinned.shscripts/ci/e2e-run.shscripts/ci/e2e-case-paths.tsvtests/e2e/cn-intervals-1-catalog.shtests/test_copy_number.pytests/test_e2e_data_cache.shbin/cn_loci.tsvbin/copy_number.pyversions.envdocs/cn-loci.mddocs/testing.mddocs/rust.mdCHANGELOG.mdHow it was checked
Retry fix head
0948aae258d8b6c383ccb2a49133d8e1bc119480: red-before-fix request sequences and green local results. The four HTTP tests use Rust and a local HTTP server. They verify immediate stops after 403/404, retry exhaustion, recovery, pin failures, cache hits and cleanup.Final-head fast checks are queued or running: Guard, Rust, Lint, Nextflow, Docs, Container Test, Personal data.
On MacMini-2 with the pinned downloads, the callability and SV builders reproduce their committed tables byte for byte (
cmp). The cn builder reproduces its committed interval rows, with the documented marker change. The--listoutput of the cn script is the same as the Python's, so the e2e data cache key does not move.Three e2e cases pass when run one by one on MacMini-2:
cn-intervals-1-catalog(4 checks),callability-1-catalog(4 checks) andfollowups-ci-3-sv-rule-regions(9 checks, including cache reuse). All three ran on0948aae2with fresh downloads. The SV case uses pcre2grep for its existing PCRE assertion because BSD grep lacks-P.cargo test -p og-dev(27 builder tests on small synthetic GTF, BED and HGNC inputs, plus four HTTP fetch tests), clippy with-D warnings, rustfmt andcargo docare clean.tests/test_copy_number.pypasses with and withoutOG.tests/test_e2e_data_cache.sh,tests/test_e2e_shards.sh,scripts/ci/e2e-scope.sh check, shellcheck and the personal-data scan pass.Red first:
1f2b7fcaplanted three wrong rules. Callability keeps the_PAR_Ycopies. The SV builder keeps the last record per HGNC id. Each cn piece ends one base short. Guard's rust job failed atcargo test. Its red run shows five failed builder tests. Reverted in 1b8998f, whose tree is the same as before the plant. Its run was cancelled when the next fix superseded it. The current-head CI run will verify the restored code. On MacMini-2 the planted og failed the cn check on 89 rows, while the two PAR plants left both real tables unchanged. On today's pins the PAR rule changes nothing. The unit tests guard it.The e2e cases' red controls passed on this head: The cn check exits 1 when one target moves by 1 kb. The callability rebuild differs from a hand-edited catalog. The SV check exits 1 and shows the MHC row when its end moves by 1.
fetch-pinned.shand the scripts: a 404 fails in 0.3 s, a wrong sha256 fails and leaves no file, a download and a cache hit both pass.Review: the upstream CodeRabbit limit was reached, so the review ran on the fork's mirror PR, build(ci): rebuild the committed asset tables from pinned downloads in Rust geiresx/OwnGenome#138. It found all four planted lines, removed by the revert. Commit
cebf6b94fixes its two other findings. Each fetch writes its own temporary file. Every attempt stops stalled transfers (30 s to connect, 1 KB/s for 60 s).ORACLE_RUNNER=local scripts/ci/oracle-diff.sh --self-testpasses all 15 checks. The comparator self-test passes all 44 cases. The Linux shadow self-test skips on macOS and remains CI's check.The latest repository CodeRabbit review was submitted at 11:59 UTC on PR #400. The shared 55-minute queue prevents another request at this handoff. The previous fork review has six resolved threads. The independent review findings are fixed.
Bead: pgp-bs9u.35