Skip to content

test(verify): run a Python operator the way the engine runs it - #8357

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

test(verify): run a Python operator the way the engine runs it#8357
kz930 wants to merge 4 commits into
apache:mainfrom
kz930:feat/verify-run-operator-python

Conversation

@kz930

@kz930 kz930 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

A Python operator is not called; it is handed to an interpreter that imports
the generated module and drives it through the same open, process and close
the engine uses. PyOpExecHarness writes that module, starts the driver, and
reads back what the operator emitted.

The driver is the engine's side of the contract written out plainly: it is
what makes the answer this side produces the engine's answer rather than an
approximation of it.

Eight lines of build.sbt are unrelated to the export and fixed while this change is in the file: File Service's test grouping read its fork options inside the lambda, which sbt hoists anyway, so the warning it printed on every build said the code meant something it did not.

Any related issues, documentation, discussions?

Part of #8325, 3 of 20; that issue lists the set in order.

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)

A Python operator is not called; it is handed to an interpreter that imports
the generated module and drives it through the same open, process and close
the engine uses. `PyOpExecHarness` writes that module, starts the driver, and
reads back what the operator emitted.

The driver is the engine's side of the contract written out plainly: it is
what makes the answer this side produces the engine's answer rather than an
approximation of it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added feature dependencies Pull requests that update a dependency file common 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: @mengw15
    You can notify them by mentioning @mengw15 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.10%. Comparing base (7976744) to head (4659852).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8357      +/-   ##
============================================
- Coverage     94.11%   94.10%   -0.02%     
- Complexity     4811     4858      +47     
============================================
  Files          1197     1203       +6     
  Lines         48813    48967     +154     
  Branches       5906     5930      +24     
============================================
+ Hits          45939    46079     +140     
+ Misses         1420     1419       -1     
- Partials       1454     1469      +15     
Flag Coverage Δ *Carryforward flag
access-control-service 81.00% <ø> (ø)
agent-service 99.32% <ø> (ø) Carriedforward from d2e8495
amber 90.08% <ø> (ø) Carriedforward from d2e8495
computing-unit-managing-service 73.67% <ø> (ø)
config-service 87.12% <ø> (+0.12%) ⬆️
file-service 87.91% <ø> (ø)
frontend 96.86% <ø> (ø) Carriedforward from d2e8495
notebook-migration-service 83.57% <ø> (+4.26%) ⬆️
pyamber 98.47% <ø> (ø) Carriedforward from d2e8495
workflow-compiling-service 77.19% <ø> (ø) Carriedforward from d2e8495

*This pull request uses carry forward flags. Click here to find out more.

☔ 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 376 0.229 25,649/34,091/34,091 us 🔴 +114.8% / 🔴 +113.3%
🔴 bs=100 sw=10 sl=64 782 0.477 126,864/156,342/156,342 us 🔴 +57.2% / 🔴 +43.1%
🔴 bs=1000 sw=10 sl=64 901 0.55 1,104,890/1,214,223/1,214,223 us 🔴 +27.8% / 🔴 +16.4%
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 376 tuples/sec 791.1 tuples/sec 755.36 tuples/sec -52.5% -50.2%
bs=10 sw=10 sl=64 MB/s 0.229 MB/s 0.483 MB/s 0.461 MB/s -52.6% -50.3%
bs=10 sw=10 sl=64 p50 25,649 us 12,219 us 12,938 us +109.9% +98.2%
bs=10 sw=10 sl=64 p95 34,091 us 15,867 us 15,980 us +114.8% +113.3%
bs=10 sw=10 sl=64 p99 34,091 us 18,101 us 19,233 us +88.3% +77.3%
bs=100 sw=10 sl=64 throughput 782 tuples/sec 1,082 tuples/sec 976.3 tuples/sec -27.8% -19.9%
bs=100 sw=10 sl=64 MB/s 0.477 MB/s 0.661 MB/s 0.596 MB/s -27.8% -20.0%
bs=100 sw=10 sl=64 p50 126,864 us 90,900 us 102,340 us +39.6% +24.0%
bs=100 sw=10 sl=64 p95 156,342 us 99,461 us 109,262 us +57.2% +43.1%
bs=100 sw=10 sl=64 p99 156,342 us 105,783 us 118,827 us +47.8% +31.6%
bs=1000 sw=10 sl=64 throughput 901 tuples/sec 1,108 tuples/sec 1,006 tuples/sec -18.6% -10.5%
bs=1000 sw=10 sl=64 MB/s 0.55 MB/s 0.676 MB/s 0.614 MB/s -18.6% -10.5%
bs=1000 sw=10 sl=64 p50 1,104,890 us 902,554 us 999,855 us +22.4% +10.5%
bs=1000 sw=10 sl=64 p95 1,214,223 us 949,861 us 1,042,833 us +27.8% +16.4%
bs=1000 sw=10 sl=64 p99 1,214,223 us 985,895 us 1,070,722 us +23.2% +13.4%
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,532.40,200,128000,376,0.229,25649.22,34091.03,34091.03
1,100,10,64,20,2557.40,2000,1280000,782,0.477,126863.98,156342.01,156342.01
2,1000,10,64,20,22191.97,20000,12800000,901,0.550,1104890.40,1214223.49,1214223.49

Eight lines this change is already in the file for. sbt lifts a `.value`
written inside a lambda to the top of the task, so the options were read once
rather than per suite either way; written where they were, they said the other
thing, and sbt warned on it in every build.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
kz930 added a commit to Nicoleee1108/texera_workflow_to_py that referenced this pull request Sep 3, 2026
Matches apache#8357, which carries this upstream. sbt lifts a `.value` written
inside a lambda to the top of the task, so the options were read once either
way; written where they were, they said the other thing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
kz930 and others added 2 commits September 3, 2026 15:11
The two helpers here were a second copy of OpExecHarness's, carrying a note that
they were kept inline so each harness read on its own and would be consolidated
if a third arrived. Two copies of the same twenty lines is already the cost that
note was deferring: they have to be changed together, and nothing says so at
either site.

Preparing the plan does not vary with the executor, so it happens once now. What
differs between the harnesses stays here: how the prepared op is run.

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

common 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.

2 participants