Repository navigation
fix(licensing): restore the ScalaPB notice on the vendored scalapb.proto - #8806
Conversation
Yicong-Huang
added
release/v1.3
Backport auto-label reportThis
|
Automated Reviewer SuggestionsBased on the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8806 +/- ##
============================================
+ Coverage 92.56% 92.61% +0.04%
- Complexity 5043 5709 +666
============================================
Files 1251 1259 +8
Lines 53358 56719 +3361
Branches 6618 7148 +530
============================================
+ Hits 49392 52529 +3137
- Misses 2319 2476 +157
- Partials 1647 1714 +67
*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 | 390 | 0.238 | 25,420/30,811/30,811 us | 🟢 -12.3% / 🔴 +130.1% |
| 🔴 | bs=100 sw=10 sl=64 | 775 | 0.473 | 125,651/160,456/160,456 us | 🔴 +8.2% / 🔴 +70.8% |
| ⚪ | bs=1000 sw=10 sl=64 | 904 | 0.552 | 1,105,182/1,153,772/1,153,772 us | ⚪ within ±5% / 🔴 -29.0% |
Baseline details
Latest main e848f94 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 390 tuples/sec | 410 tuples/sec | 962.17 tuples/sec | -4.9% | -59.5% |
| bs=10 sw=10 sl=64 | MB/s | 0.238 MB/s | 0.25 MB/s | 0.587 MB/s | -4.8% | -59.5% |
| bs=10 sw=10 sl=64 | p50 | 25,420 us | 24,684 us | 11,046 us | +3.0% | +130.1% |
| bs=10 sw=10 sl=64 | p95 | 30,811 us | 35,127 us | 13,484 us | -12.3% | +128.5% |
| bs=10 sw=10 sl=64 | p99 | 30,811 us | 35,127 us | 16,626 us | -12.3% | +85.3% |
| bs=100 sw=10 sl=64 | throughput | 775 tuples/sec | 810 tuples/sec | 1,234 tuples/sec | -4.3% | -37.2% |
| bs=100 sw=10 sl=64 | MB/s | 0.473 MB/s | 0.494 MB/s | 0.753 MB/s | -4.3% | -37.2% |
| bs=100 sw=10 sl=64 | p50 | 125,651 us | 122,320 us | 87,952 us | +2.7% | +42.9% |
| bs=100 sw=10 sl=64 | p95 | 160,456 us | 148,359 us | 93,929 us | +8.2% | +70.8% |
| bs=100 sw=10 sl=64 | p99 | 160,456 us | 148,359 us | 103,096 us | +8.2% | +55.6% |
| bs=1000 sw=10 sl=64 | throughput | 904 tuples/sec | 919 tuples/sec | 1,272 tuples/sec | -1.6% | -29.0% |
| bs=1000 sw=10 sl=64 | MB/s | 0.552 MB/s | 0.561 MB/s | 0.777 MB/s | -1.6% | -28.9% |
| bs=1000 sw=10 sl=64 | p50 | 1,105,182 us | 1,087,260 us | 862,437 us | +1.6% | +28.1% |
| bs=1000 sw=10 sl=64 | p95 | 1,153,772 us | 1,163,684 us | 904,996 us | -0.9% | +27.5% |
| bs=1000 sw=10 sl=64 | p99 | 1,153,772 us | 1,163,684 us | 929,784 us | -0.9% | +24.1% |
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,513.20,200,128000,390,0.238,25420.22,30810.83,30810.83
1,100,10,64,20,2579.44,2000,1280000,775,0.473,125650.98,160455.65,160455.65
2,1000,10,64,20,22129.01,20000,12800000,904,0.552,1105182.31,1153771.92,1153771.92
xuang7
left a comment
xuang7
left a comment
There was a problem hiding this comment.
LGTM on the overall approach. Upstream ships the file without a header, and ScalaPB does not appear to have a NOTICE file.
One suggestion regarding the header: Apache RAT 0.16.1, which is used in our RC verification guide, does not seem to recognize the current three-line notice and reports 1 Unknown Licenses for this file.
# Conflicts: # .licenserc.yaml # LICENSE
…11.4 Five option lines carried `[default = x]` where every ScalaPB release writes `[default=x]`; the spacing arrived in apache#3182, which resolved a duplicate copy in favour of the reformatted one. Restoring it makes the file match the source the header cites, so the provenance note can be checked with a plain diff and the Apache-2.0 modified-file notice does not apply. Protobuf ignores the whitespace, and this is the form ScalaPB itself ships.
What changes were proposed in this PR?
Per the note at the end of #8802,
common/workflow-core/src/main/protobuf/scalapb/scalapb.protois vendoredfrom ScalaPB (Apache-2.0) but carried the standard ASF header — the same
class of problem as #8775. Mirroring that fix:
scalapb.proto: ASF header → a vendored-from notice (upstream ships thefile with no header; its license is the repository-level Apache-2.0, so no
separate license copy is needed);
LICENSE: a ScalaPB entry next to the other Apache-2.0 third-partyentries ("this LICENSE file");
.licenserc.yaml: exempt the file from the ASF-header check.Comment-only change to the proto; protoc output is unaffected. This addresses
only the vendored-file item in #8802 — the build-time-generation proposal
stays open there.
Any related issues, documentation, discussions?
Addresses the vendored scalapb.proto item in #8802. Same pattern as #8775/#8776.
How was this PR tested?
License-metadata-only change; the Check License Headers CI job verifies the
exemption. The file is identical on main and both release branches.
Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (claude-fable-5)