Skip to content

ci: check every GitHub-read YAML file for duplicate keys - #8625

Closed
mengw15 wants to merge 1 commit into
apache:mainfrom
mengw15:ci/strict-yaml-parse-check
Closed

mengw15 wants to merge 1 commit into
apache:mainfrom
mengw15:ci/strict-yaml-parse-check

Conversation

@mengw15

@mengw15 mengw15 commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

A duplicate mapping key is invalid YAML, but PyYAML keeps the last one and says nothing:

>>> yaml.safe_load("jobs:\n  a: {}\njobs:\n  b: {}")
{'jobs': {'b': {}}}

A local parse therefore cannot see the mistake, while GitHub's loader rejects the file. Both halves of that gap are silent: the change looks fine before it merges, and afterwards the workflow simply stops starting rather than failing.

.github/scripts/test_ci_yaml.sh loads .asf.yaml and every .yml/.yaml under .github/ (30 files today) with a duplicate-key-strict loader. Beyond the workflows, the files worth naming are .github/release-branches.yml, where a duplicated key drops a release branch and with it every backport that branch would have been nominated for, and .github/labeler.yml, where it stops a rule matching without anyone noticing.

No change to build.yml: its infra job runs every .github/scripts/test_*.sh it finds, after installing amber/dev-requirements.txt, which already pins PyYAML.

Any related issues, documentation, discussions?

#8379 added this check as one of two invariants in .github/scripts/test_asf_rulesets.sh. The other one described a ruleset split GitHub refuses to create and goes away with #8624's revert; this keeps the half that still has a subject, widened beyond workflows and .asf.yaml.

How was this PR tested?

Green on the current tree (30 files). The failure path was verified on one file of each kind — a workflow, release-branches.yml, labeler.yml and .asf.yaml — by appending a duplicated key to each in turn: every one reports the file, the key and the line, and exits 1. The tree is unchanged afterwards.

Was this PR authored or co-authored using generative AI tooling?

Yes. Generated-by: Claude Code (claude-opus-5)

A duplicate mapping key is invalid YAML, but PyYAML keeps the last one and
says nothing, so a local parse cannot see the mistake while GitHub's loader
rejects the file. The gap is silent in both directions: the change looks
fine locally, and on GitHub the workflow simply stops starting.

apache#8379 introduced this check as one of two invariants in
test_asf_rulesets.sh; the other one was about the ruleset split that
GitHub refuses to create, and goes away with it in the revert. This keeps
the half that has a subject, and widens it from workflows plus .asf.yaml
to every YAML file GitHub reads here -- .github/labeler.yml, the issue
templates and .github/release-branches.yml, where a duplicated key would
drop a release branch from the backport nomination entirely.

Picked up automatically by build.yml's infra job, which runs every
.github/scripts/test_*.sh after installing amber/dev-requirements.txt.
@github-actions github-actions Bot added the ci changes related to CI label Sep 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • No candidates found from git blame history.

@mengw15
mengw15 marked this pull request as draft September 21, 2026 07:37
@mengw15
mengw15 marked this pull request as ready for review September 21, 2026 07:39
@mengw15 mengw15 closed this Sep 21, 2026
@codecov-commenter

codecov-commenter commented Sep 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.80%. Comparing base (279e12b) to head (46866d9).

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #8625   +/-   ##
=========================================
  Coverage     92.80%   92.80%           
  Complexity     4898     4898           
=========================================
  Files          1239     1239           
  Lines         52252    52252           
  Branches       6425     6425           
=========================================
  Hits          48492    48492           
- Misses         2183     2184    +1     
+ Partials       1577     1576    -1     
Flag Coverage Δ
access-control-service 71.78% <ø> (ø)
agent-service 99.16% <ø> (ø)
amber 88.59% <ø> (-0.01%) ⬇️
computing-unit-managing-service 55.20% <ø> (ø)
config-service 87.37% <ø> (ø)
file-service 81.53% <ø> (ø)
frontend 96.64% <ø> (+<0.01%) ⬆️
notebook-migration-service 83.73% <ø> (ø)
pyamber 98.48% <ø> (ø)
workflow-compiling-service 74.09% <ø> (ø)

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

🟢 0 better · 🔴 7 worse · ⚪ 8 noise (<±5%) · 0 without baseline

Compared against main 279e12b benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

config throughput MB/s latency max Δ latest / 7d
🔴 bs=10 sw=10 sl=64 441 0.269 21,157/38,281/38,281 us 🔴 +23.6% / 🔴 +142.3%
🔴 bs=100 sw=10 sl=64 945 0.577 100,668/141,752/141,752 us 🔴 +14.4% / 🔴 +30.4%
bs=1000 sw=10 sl=64 1,153 0.704 862,524/921,807/921,807 us ⚪ within ±5% / 🟢 -13.7%
Baseline details

Latest main 279e12b from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 441 tuples/sec 482 tuples/sec 767.43 tuples/sec -8.5% -42.5%
bs=10 sw=10 sl=64 MB/s 0.269 MB/s 0.294 MB/s 0.468 MB/s -8.5% -42.6%
bs=10 sw=10 sl=64 p50 21,157 us 20,058 us 12,880 us +5.5% +64.3%
bs=10 sw=10 sl=64 p95 38,281 us 30,962 us 15,801 us +23.6% +142.3%
bs=10 sw=10 sl=64 p99 38,281 us 30,962 us 19,767 us +23.6% +93.7%
bs=100 sw=10 sl=64 throughput 945 tuples/sec 983 tuples/sec 988.73 tuples/sec -3.9% -4.4%
bs=100 sw=10 sl=64 MB/s 0.577 MB/s 0.6 MB/s 0.603 MB/s -3.8% -4.4%
bs=100 sw=10 sl=64 p50 100,668 us 98,806 us 102,684 us +1.9% -2.0%
bs=100 sw=10 sl=64 p95 141,752 us 123,878 us 108,712 us +14.4% +30.4%
bs=100 sw=10 sl=64 p99 141,752 us 123,878 us 118,731 us +14.4% +19.4%
bs=1000 sw=10 sl=64 throughput 1,153 tuples/sec 1,145 tuples/sec 1,022 tuples/sec +0.7% +12.8%
bs=1000 sw=10 sl=64 MB/s 0.704 MB/s 0.699 MB/s 0.624 MB/s +0.7% +12.8%
bs=1000 sw=10 sl=64 p50 862,524 us 871,466 us 999,086 us -1.0% -13.7%
bs=1000 sw=10 sl=64 p95 921,807 us 913,093 us 1,037,033 us +1.0% -11.1%
bs=1000 sw=10 sl=64 p99 921,807 us 913,093 us 1,066,123 us +1.0% -13.5%
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,453.21,200,128000,441,0.269,21156.91,38281.32,38281.32
1,100,10,64,20,2116.38,2000,1280000,945,0.577,100667.87,141752.04,141752.04
2,1000,10,64,20,17343.67,20000,12800000,1153,0.704,862524.21,921807.10,921807.10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci changes related to CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants