Repository navigation
feat(workflow-operator): read the generated module as UTF-8 in three test drivers - #8988
Conversation
…drivers The spec writes the module as UTF-8, and open() without an encoding decodes it with the locale's, which is cp1252 on Windows. Closes apache#8976 Co-Authored-By: Claude Opus 5.5 <[email protected]>
Automated Reviewer SuggestionsBased on the
|
Linked issue already claimedThis PR closes an issue that someone else has already claimed:
Please check with them before this merges. Merging closes the issue, and any work they have in progress is left with nothing to fix.
Refreshed whenever this PR is edited; deleted once nothing here applies. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8988 +/- ##
============================================
- Coverage 92.61% 92.61% -0.01%
+ Complexity 5059 5058 -1
============================================
Files 1258 1258
Lines 53780 53780
Branches 6707 6707
============================================
- Hits 49811 49809 -2
- Misses 2313 2314 +1
- Partials 1656 1657 +1
*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:
|
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 372 | 0.227 | 26,487/32,889/32,889 us | 🔴 +14.7% / 🔴 +118.4% |
| 🟢 | bs=100 sw=10 sl=64 | 780 | 0.476 | 126,351/144,256/144,256 us | 🟢 -10.0% / 🔴 +38.5% |
| 🔴 | bs=1000 sw=10 sl=64 | 910 | 0.556 | 1,099,987/1,181,077/1,181,077 us | 🔴 +5.9% / 🔴 +19.9% |
Baseline details
Latest main 6a29c59 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 372 tuples/sec | 416 tuples/sec | 822.11 tuples/sec | -10.6% | -54.8% |
| bs=10 sw=10 sl=64 | MB/s | 0.227 MB/s | 0.254 MB/s | 0.502 MB/s | -10.6% | -54.8% |
| bs=10 sw=10 sl=64 | p50 | 26,487 us | 23,097 us | 12,129 us | +14.7% | +118.4% |
| bs=10 sw=10 sl=64 | p95 | 32,889 us | 36,357 us | 15,469 us | -9.5% | +112.6% |
| bs=10 sw=10 sl=64 | p99 | 32,889 us | 36,357 us | 18,357 us | -9.5% | +79.2% |
| bs=100 sw=10 sl=64 | throughput | 780 tuples/sec | 804 tuples/sec | 1,058 tuples/sec | -3.0% | -26.3% |
| bs=100 sw=10 sl=64 | MB/s | 0.476 MB/s | 0.491 MB/s | 0.646 MB/s | -3.1% | -26.3% |
| bs=100 sw=10 sl=64 | p50 | 126,351 us | 122,623 us | 97,399 us | +3.0% | +29.7% |
| bs=100 sw=10 sl=64 | p95 | 144,256 us | 160,241 us | 104,184 us | -10.0% | +38.5% |
| bs=100 sw=10 sl=64 | p99 | 144,256 us | 160,241 us | 117,615 us | -10.0% | +22.7% |
| bs=1000 sw=10 sl=64 | throughput | 910 tuples/sec | 932 tuples/sec | 1,099 tuples/sec | -2.4% | -17.2% |
| bs=1000 sw=10 sl=64 | MB/s | 0.556 MB/s | 0.569 MB/s | 0.671 MB/s | -2.3% | -17.1% |
| bs=1000 sw=10 sl=64 | p50 | 1,099,987 us | 1,070,663 us | 946,743 us | +2.7% | +16.2% |
| bs=1000 sw=10 sl=64 | p95 | 1,181,077 us | 1,115,452 us | 985,437 us | +5.9% | +19.9% |
| bs=1000 sw=10 sl=64 | p99 | 1,181,077 us | 1,115,452 us | 1,012,622 us | +5.9% | +16.6% |
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,537.64,200,128000,372,0.227,26486.87,32888.69,32888.69
1,100,10,64,20,2564.22,2000,1280000,780,0.476,126350.85,144256.30,144256.30
2,1000,10,64,20,21973.02,20000,12800000,910,0.556,1099986.52,1181076.82,1181076.82
|
@carloea2 Could you review this? The three test drivers left out of the earlier UTF-8 fix now read the generated module with |
carloea2
left a comment
carloea2
left a comment
There was a problem hiding this comment.
Verified the three encoding changes against current upstream main on Windows. All 51 tests in the three affected specs and scalafmtCheckAll pass. CI is green.
Command: sbt -batch "WorkflowOperator/testOnly *MachineLearningScorerOpDescSpec *FilledAreaPlotOpDescSpec *TreePlotOpDescSpec" scalafmtCheckAll
What changes were proposed in this PR?
The Python drivers in
MachineLearningScorerOpDescSpec,FilledAreaPlotOpDescSpecandTreePlotOpDescSpecopened the generated module without an encoding, so Python decoded it with the locale's, which is cp1252 on Windows. The spec writes it as UTF-8. Each driver now passesencoding="utf-8", as the drivers that read Plotly HTML already do.Any related issues, documentation, discussions?
Closes #8976
How was this PR tested?
The three specs pass locally (51 tests). The generated modules are ASCII today, so the change only matters once one is not.
Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5.5)
🤖 Generated with Claude Code