Skip to content

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

Merged
aglinxinyuan merged 7 commits into
apache:mainfrom
kz930:feat/verify-run-operator-in-engine
Sep 15, 2026
Merged

aglinxinyuan merged 7 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, 3 of 27; 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 93.63%. Comparing base (4d7fd49) to head (fe8dff0).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8356      +/-   ##
============================================
+ Coverage     93.62%   93.63%   +0.01%     
  Complexity     4842     4842              
============================================
  Files          1211     1211              
  Lines         49991    49991              
  Branches       6125     6125              
============================================
+ Hits          46805    46811       +6     
+ Misses         1675     1672       -3     
+ Partials       1511     1508       -3     
Flag Coverage Δ
access-control-service 80.18% <ø> (ø)
agent-service 99.32% <ø> (ø)
amber 89.86% <ø> (+0.01%) ⬆️
computing-unit-managing-service 77.14% <ø> (ø)
config-service 87.12% <ø> (ø)
file-service 83.65% <ø> (ø)
frontend 96.16% <ø> (+<0.01%) ⬆️
notebook-migration-service 83.73% <ø> (ø)
pyamber 98.47% <ø> (+0.06%) ⬆️
workflow-compiling-service 75.00% <ø> (ø)

☔ 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 · 🔴 9 worse · ⚪ 6 noise (<±5%) · 0 without baseline

Compared against main 4d7fd49 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 382 0.233 25,745/33,986/33,986 us 🔴 +25.7% / 🔴 +110.7%
🔴 bs=100 sw=10 sl=64 945 0.577 104,993/135,798/135,798 us 🔴 +18.2% / 🔴 +24.9%
🔴 bs=1000 sw=10 sl=64 1,081 0.66 908,149/1,064,685/1,064,685 us 🔴 +13.2% / 🟢 -8.9%
Baseline details

Latest main 4d7fd49 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 382 tuples/sec 461 tuples/sec 755.28 tuples/sec -17.1% -49.4%
bs=10 sw=10 sl=64 MB/s 0.233 MB/s 0.281 MB/s 0.461 MB/s -17.1% -49.5%
bs=10 sw=10 sl=64 p50 25,745 us 20,484 us 12,957 us +25.7% +98.7%
bs=10 sw=10 sl=64 p95 33,986 us 29,676 us 16,134 us +14.5% +110.7%
bs=10 sw=10 sl=64 p99 33,986 us 29,676 us 20,333 us +14.5% +67.1%
bs=100 sw=10 sl=64 throughput 945 tuples/sec 976 tuples/sec 980.1 tuples/sec -3.2% -3.6%
bs=100 sw=10 sl=64 MB/s 0.577 MB/s 0.596 MB/s 0.598 MB/s -3.2% -3.5%
bs=100 sw=10 sl=64 p50 104,993 us 103,308 us 101,894 us +1.6% +3.0%
bs=100 sw=10 sl=64 p95 135,798 us 114,893 us 108,718 us +18.2% +24.9%
bs=100 sw=10 sl=64 p99 135,798 us 114,893 us 122,482 us +18.2% +10.9%
bs=1000 sw=10 sl=64 throughput 1,081 tuples/sec 1,102 tuples/sec 1,011 tuples/sec -1.9% +6.9%
bs=1000 sw=10 sl=64 MB/s 0.66 MB/s 0.673 MB/s 0.617 MB/s -1.9% +6.9%
bs=1000 sw=10 sl=64 p50 908,149 us 909,443 us 996,422 us -0.1% -8.9%
bs=1000 sw=10 sl=64 p95 1,064,685 us 940,438 us 1,037,670 us +13.2% +2.6%
bs=1000 sw=10 sl=64 p99 1,064,685 us 940,438 us 1,072,152 us +13.2% -0.7%
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,522.99,200,128000,382,0.233,25744.62,33986.17,33986.17
1,100,10,64,20,2117.03,2000,1280000,945,0.577,104992.72,135797.72,135797.72
2,1000,10,64,20,18495.01,20000,12800000,1081,0.660,908148.99,1064684.51,1064684.51

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.

@kz930

kz930 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

@aglinxinyuan may you take a look at this?

@kz930

kz930 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

/request-review @mengw15

@github-actions
github-actions Bot requested a review from mengw15 September 4, 2026 22:31
kz930 and others added 2 commits September 8, 2026 23:40
The wiring is read from getPhysicalPlan, which is why a join, a split and a
source all work; the enumeration ahead of that said the same thing twice.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It restated its own signature and listed a codec table that lives in
TupleIO. The reasons a reader cannot derive stay.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@mengw15
mengw15 removed their request for review September 11, 2026 00:19
@kz930

kz930 commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

/request-review @aglinxinyuan

…rator-in-engine

# Conflicts:
#	workflow-compiling-service/build.sbt

@aglinxinyuan aglinxinyuan 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.

LGTM!

@aglinxinyuan
aglinxinyuan added this pull request to the merge queue Sep 14, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 14, 2026
@kz930

kz930 commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

@aglinxinyuan could you re-add this to the merge queue? It was dropped because Docker Hub refused the MinIO testcontainer pull (pull access denied for minio/minio), so the S3 suites aborted in amber and file-service without any test actually failing.

@aglinxinyuan
aglinxinyuan added this pull request to the merge queue Sep 15, 2026
Merged via the queue into apache:main with commit bf356f5 Sep 15, 2026
39 checks passed
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

4 participants