Skip to content

test(verify): run an operator through the engine and keep what it wrote - #8356

Open
kz930 wants to merge 4 commits into
apache:mainfrom
kz930:feat/verify-run-operator-in-engine
Open

test(verify): run an operator through the engine and keep what it wrote#8356
kz930 wants to merge 4 commits into
apache:mainfrom
kz930:feat/verify-run-operator-in-engine

Conversation

@kz930

@kz930 kz930 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

An operator's answer cannot be compared against anything until there is a way
to get one. OpExecHarness builds the physical operator a descriptor
describes, feeds it the rows of a JSONL file per input port, and writes what
each output port produced back out, schema in a sidecar because JSONL carries
values alone and cannot say a column is an integer rather than a number.

The tests that need a Python interpreter are tagged and split into a job that
provisions one, so the job that does not stays as fast as it was.

Any related issues, documentation, discussions?

Part of #8325, 2 of 21; that issue lists the set in order.

Closes #8408, the task this change is the whole of.

How was this PR tested?

The tests in this change cover it. The whole set is exercised together once the last piece lands: every operator run through the engine and through its generated script, and the two answers compared.

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

Generated-by: Claude Code (Opus 5)

An operator's answer cannot be compared against anything until there is a way
to get one. `OpExecHarness` builds the physical operator a descriptor
describes, feeds it the rows of a JSONL file per input port, and writes what
each output port produced back out, schema in a sidecar because JSONL carries
values alone and cannot say a column is an integer rather than a number.

The tests that need a Python interpreter are tagged and split into a job that
provisions one, so the job that does not stays as fast as it was.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added feature ci changes related to CI platform Non-amber Scala service paths labels Sep 2, 2026
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

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

  • Contributors with relevant context: @aglinxinyuan
    You can notify them by mentioning @aglinxinyuan in a comment.

@codecov-commenter

codecov-commenter commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.02%. Comparing base (7976744) to head (24ab82e).
⚠️ Report is 13 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8356      +/-   ##
============================================
- Coverage     94.11%   94.02%   -0.09%     
- Complexity     4811     4819       +8     
============================================
  Files          1197     1204       +7     
  Lines         48813    48991     +178     
  Branches       5906     5956      +50     
============================================
+ Hits          45939    46064     +125     
- Misses         1420     1459      +39     
- Partials       1454     1468      +14     
Flag Coverage Δ
access-control-service 81.00% <ø> (ø)
agent-service 99.32% <ø> (ø)
amber 89.90% <ø> (-0.18%) ⬇️
computing-unit-managing-service 73.67% <ø> (ø)
config-service 87.12% <ø> (+0.12%) ⬆️
file-service 87.91% <ø> (ø)
frontend 96.79% <ø> (-0.08%) ⬇️
notebook-migration-service 83.57% <ø> (+4.26%) ⬆️
pyamber 98.47% <ø> (ø)
workflow-compiling-service 77.19% <ø> (ø)

☔ 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

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

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

CI benchmark results are noisy; treat <±5% as noise unless repeated.

Dashboard · Run

config throughput MB/s latency max Δ latest / 7d
🔴 bs=10 sw=10 sl=64 351 0.214 26,316/38,662/38,662 us 🔴 +143.7% / 🔴 +141.9%
🔴 bs=100 sw=10 sl=64 785 0.479 125,708/147,477/147,477 us 🔴 +48.3% / 🔴 +35.0%
🔴 bs=1000 sw=10 sl=64 866 0.529 1,153,154/1,247,231/1,247,231 us 🔴 +31.3% / 🔴 +19.6%
Baseline details

Latest main cd4fd5a from 2026-09-03T13:02:32.113Z

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 351 tuples/sec 791.1 tuples/sec 755.36 tuples/sec -55.6% -53.5%
bs=10 sw=10 sl=64 MB/s 0.214 MB/s 0.483 MB/s 0.461 MB/s -55.7% -53.6%
bs=10 sw=10 sl=64 p50 26,316 us 12,219 us 12,938 us +115.4% +103.4%
bs=10 sw=10 sl=64 p95 38,662 us 15,867 us 15,980 us +143.7% +141.9%
bs=10 sw=10 sl=64 p99 38,662 us 18,101 us 19,233 us +113.6% +101.0%
bs=100 sw=10 sl=64 throughput 785 tuples/sec 1,082 tuples/sec 976.3 tuples/sec -27.5% -19.6%
bs=100 sw=10 sl=64 MB/s 0.479 MB/s 0.661 MB/s 0.596 MB/s -27.5% -19.6%
bs=100 sw=10 sl=64 p50 125,708 us 90,900 us 102,340 us +38.3% +22.8%
bs=100 sw=10 sl=64 p95 147,477 us 99,461 us 109,262 us +48.3% +35.0%
bs=100 sw=10 sl=64 p99 147,477 us 105,783 us 118,827 us +39.4% +24.1%
bs=1000 sw=10 sl=64 throughput 866 tuples/sec 1,108 tuples/sec 1,006 tuples/sec -21.8% -13.9%
bs=1000 sw=10 sl=64 MB/s 0.529 MB/s 0.676 MB/s 0.614 MB/s -21.7% -13.9%
bs=1000 sw=10 sl=64 p50 1,153,154 us 902,554 us 999,855 us +27.8% +15.3%
bs=1000 sw=10 sl=64 p95 1,247,231 us 949,861 us 1,042,833 us +31.3% +19.6%
bs=1000 sw=10 sl=64 p99 1,247,231 us 985,895 us 1,070,722 us +26.5% +16.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,569.15,200,128000,351,0.214,26315.80,38661.51,38661.51
1,100,10,64,20,2546.94,2000,1280000,785,0.479,125707.83,147476.83,147476.83
2,1000,10,64,20,23081.98,20000,12800000,866,0.529,1153153.94,1247230.83,1247230.83

kz930 and others added 3 commits September 3, 2026 15:08
Both harnesses run the same physical plan; only the executor behind it differs.
Preparing that plan is the same work either way: propagate schemas at the
external input ports, and refuse a fixture set that does not cover them.

Widening the two to `private[verify]` lets the Python harness call them instead
of carrying a second copy that has to be kept in step by hand.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three kinds of comment came out. A drawing of the string the code below
assembles. A restatement of a branch the reader can see. And the word MVP,
which dated the scope to a moment rather than stating it.

What replaces them says the same thing shorter, or says what the code cannot:
which cases the harness does not drive and why none of them has an operator
asking for it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The filter routes the Python-forking specs to the job that provisions Python and
keeps them out of the fast unit job. It selects on `IntegrationTest`, which this
change introduces, so the two belong together: configured any earlier it selects
on a tag nothing carries.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the dependencies Pull requests that update a dependency file label Sep 3, 2026

@carloea2 carloea2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The engine harness looks good.

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

Labels

ci changes related to CI dependencies Pull requests that update a dependency file feature platform Non-amber Scala service paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Run an operator through the engine and keep what it wrote

3 participants