ci: discover staged Codecov flags instead of a hardcoded matrix#6824
Merged
aglinxinyuan merged 3 commits intoJul 23, 2026
Merged
Conversation
Follow-up to apache#6730. The deferred Codecov Upload matrix hardcoded all 11 flags and tried to download each from the source run. On a label-gated PR (e.g. a frontend-only PR that only stages codecov-frontend) the other 10 legs hit "Artifact not found" — noise that looks like a name mismatch — and because the download used continue-on-error, a genuinely failed download was swallowed, the leg went green, and notify-failure never fired. Add a discover job that lists the source run's actual codecov-* artifacts and drives the upload matrix via fromJSON, and drop continue-on-error on the download. Now only the flags that were actually built are processed (no spurious errors), and a listed artifact that fails to download/upload fails the leg so notify-failure fires. Also removes the hardcoded flag list, so adding a platform service no longer needs a matching row here (build.yml comment updated). Harden the discover github-script to read the source run id from env rather than interpolating a workflow_dispatch-supplied run_id into the script body.
Contributor
Automated Reviewer SuggestionsBased on the
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the deferred Codecov Upload GitHub Actions workflow by dynamically discovering which codecov-* artifacts were actually produced by the upstream “Required Checks” run, then uploading only those flags—eliminating spurious “artifact not found” errors and ensuring real download/upload failures surface correctly.
Changes:
- Add a
discoverjob to listcodecov-*artifacts from the source run and drive the upload matrix viafromJSON. - Remove
continue-on-errorfrom artifact download so missing/failed downloads correctly fail the corresponding matrix leg and trigger failure notification. - Update
build.ymlcomments to reflect that Codecov flags are now discovered dynamically (no hardcoded matrix sync).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
.github/workflows/codecov-upload.yml |
Introduces discover job and switches upload matrix to dynamic flags; makes download failures fail loudly and adjusts failure notification dependencies. |
.github/workflows/build.yml |
Updates staging/upload comment to reflect dynamic Codecov artifact discovery (no manual flag matrix maintenance). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Per Copilot review on apache#6824: - Validate the source run id in the discover script and fail with a clear message instead of letting a non-numeric workflow_dispatch run_id become NaN and produce a confusing artifacts-API error. - Default the upload matrix to `[]` (fromJSON(flags || '[]')) so an empty-string output can never throw during matrix evaluation; the job `if` still skips the empty case.
Contributor
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🟢 | bs=10 sw=10 sl=64 | 481 | 0.293 | 20,605/27,906/27,906 us | 🟢 -7.4% / 🔴 +73.1% |
| 🔴 | bs=100 sw=10 sl=64 | 913 | 0.558 | 105,402/145,488/145,488 us | 🔴 +13.2% / 🔴 +35.5% |
| ⚪ | bs=1000 sw=10 sl=64 | 1,125 | 0.687 | 887,709/907,468/907,468 us | ⚪ within ±5% / 🟢 -14.1% |
Baseline details
Latest main 0ba5d1a from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 481 tuples/sec | 472 tuples/sec | 760.2 tuples/sec | +1.9% | -36.7% |
| bs=10 sw=10 sl=64 | MB/s | 0.293 MB/s | 0.288 MB/s | 0.464 MB/s | +1.7% | -36.9% |
| bs=10 sw=10 sl=64 | p50 | 20,605 us | 22,240 us | 12,781 us | -7.4% | +61.2% |
| bs=10 sw=10 sl=64 | p95 | 27,906 us | 30,107 us | 16,118 us | -7.3% | +73.1% |
| bs=10 sw=10 sl=64 | p99 | 27,906 us | 30,107 us | 19,074 us | -7.3% | +46.3% |
| bs=100 sw=10 sl=64 | throughput | 913 tuples/sec | 978 tuples/sec | 988.68 tuples/sec | -6.6% | -7.7% |
| bs=100 sw=10 sl=64 | MB/s | 0.558 MB/s | 0.597 MB/s | 0.603 MB/s | -6.5% | -7.5% |
| bs=100 sw=10 sl=64 | p50 | 105,402 us | 99,562 us | 101,448 us | +5.9% | +3.9% |
| bs=100 sw=10 sl=64 | p95 | 145,488 us | 128,498 us | 107,410 us | +13.2% | +35.5% |
| bs=100 sw=10 sl=64 | p99 | 145,488 us | 128,498 us | 120,258 us | +13.2% | +21.0% |
| bs=1000 sw=10 sl=64 | throughput | 1,125 tuples/sec | 1,131 tuples/sec | 1,023 tuples/sec | -0.5% | +10.0% |
| bs=1000 sw=10 sl=64 | MB/s | 0.687 MB/s | 0.69 MB/s | 0.624 MB/s | -0.4% | +10.0% |
| bs=1000 sw=10 sl=64 | p50 | 887,709 us | 883,915 us | 983,888 us | +0.4% | -9.8% |
| bs=1000 sw=10 sl=64 | p95 | 907,468 us | 923,691 us | 1,030,100 us | -1.8% | -11.9% |
| bs=1000 sw=10 sl=64 | p99 | 907,468 us | 923,691 us | 1,056,513 us | -1.8% | -14.1% |
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,416.05,200,128000,481,0.293,20604.62,27906.22,27906.22
1,100,10,64,20,2189.46,2000,1280000,913,0.558,105402.02,145487.57,145487.57
2,1000,10,64,20,17770.87,20000,12800000,1125,0.687,887709.45,907468.36,907468.36
Yicong-Huang
approved these changes
Jul 23, 2026
Address review (apache#6824): unlike the upload job's download (which fails hard so a real failure surfaces), notify-failure's download is only fetching a codecov-* artifact to read the PR number for the comment. Keep continue-on-error so a missing artifact degrades to "no comment" via the `if: pr != ''` guard instead of hard-failing notify-failure — which isn't a required check, so failing it would be invisible noise. Comment-only change.
aglinxinyuan
enabled auto-merge
July 23, 2026 09:02
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6824 +/- ##
=========================================
Coverage 76.28% 76.28%
Complexity 3452 3452
=========================================
Files 1161 1161
Lines 45916 45916
Branches 5099 5099
=========================================
Hits 35025 35025
Misses 9327 9327
Partials 1564 1564
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Jul 23, 2026
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.
What changes were proposed in this PR?
Follow-up to #6730. The deferred
Codecov Uploadworkflow hardcoded all 11 flags in its matrix and tried to download each from the source run. On a label-gated PR — e.g. a frontend-only PR that only stagescodecov-frontend— the other 10 legs hit##[error] Artifact not found for name: codecov-<flag>, which looks like a name mismatch; and because the download usedcontinue-on-error: true, a genuinely failed download was swallowed, the leg went green, andnotify-failurenever fired.Fix: add a
discoverjob that lists the source run's actualcodecov-*artifacts and drives the upload matrix from that (fromJSON), and dropcontinue-on-erroron the download.notify-failurefires (previously it went silently green).platformservice no longer needs a matching matrix row here.Observed on the first live run after #6730 merged (run 29984668372, triggered by a frontend-only PR's checks):
frontenduploaded fine, but 10 legs loggedArtifact not foundfor the label-gated flags and the run still went green.Any related issues, documentation, discussions?
Follow-up to #6730 (reported on #6685).
How was this PR tested?
workflow_runonly activates from the copy of the file on the default branch, so this can't run on its own PR. Validated by YAML lint and an adversarial review of the discover → upload → notify flow: empty-flags cleanly skips (no failure, no notify), the dynamicfromJSONmatrix handles one or many flags, a download/upload failure now fails the leg and firesnotify-failure,amber-integration(results-only) skips the coverage step viahashFiles, and theworkflow_dispatchpath resolves the run id.workflow_dispatch(run_id) entry remains for manual post-merge validation against a finished Required Checks run.Was this PR authored or co-authored using generative AI tooling?
Yes.
Generated-by: Claude Code (Opus 4.8 [1M context])