refactor: untrack generated python proto, regenerate in build by Ma77Ball · Pull Request #5077 · apache/texera

Ma77Ball · 2026-05-15T11:18:30Z

What changes were proposed in this PR?

Stop version-tracking the betterproto-generated Python files under amber/src/main/python/proto/. Add a genPythonProto sbt task that runs bin/python-proto-gen.sh as a dependency of WorkflowExecutionService/Compile/compile, with a graceful skip when protoc is missing. Wire protoc 3.19.4 (matching PB.protocVersion) and betterproto[compiler]==2.0.0b7 into the three image build stages (worker,master, web-app) and the three CI jobs that touch Python proto (python, python-state-materialization-mac, amber-integration). The 14th tracked file (proto/org/apache/texera/web/__init__.py) had no corresponding .proto source and is gone after regen; grep confirms no callers.

Any related issues, documentation, or discussions?

Closes: #4102

How was this PR tested?

locally: sbt clean WorkflowExecutionService/compile regenerates the tree and pytest -m "not integration" passes; skip path verified with protoc absent. All three Docker images build end-to-end. CI covers the workflow YAML on
push.

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

Co-authored with Claude Opus 4.7 in compliance with ASF

Ma77Ball · 2026-05-15T11:18:49Z

/request-review @Yicong-Huang

codecov-commenter · 2026-05-15T11:19:29Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 43.83%. Comparing base (0f5f791) to head (32c268a).

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #5077      +/-   ##
============================================
+ Coverage     43.66%   43.83%   +0.17%     
  Complexity     2218     2218              
============================================
  Files          1049     1051       +2     
  Lines         40580    40705     +125     
  Branches       4324     4324              
============================================
+ Hits          17719    17843     +124     
  Misses        21766    21766              
- Partials       1095     1096       +1

Flag	Coverage Δ		*Carryforward flag
access-control-service	`39.53% <ø> (ø)`
agent-service	`33.76% <ø> (ø)`
amber	`43.94% <ø> (ø)`		Carriedforward from 0f5f791
computing-unit-managing-service	`1.38% <ø> (ø)`
config-service	`19.35% <ø> (ø)`
file-service	`32.18% <ø> (ø)`
frontend	`35.15% <ø> (ø)`
python	`90.79% <ø> (+0.28%)`	⬆️
workflow-compiling-service	`58.39% <ø> (ø)`

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Yicong-Huang · 2026-05-15T15:42:47Z

Thanks, that's a good direction for change! Let inline comments.

Please make pr description concise. The "how you test" section is basically saying "ci" can verify. I don't get too much new info reading it.

…o feat/changeProtobufGenerate

Ma77Ball · 2026-05-22T09:07:07Z

@Yicong-Huang, I addressed the comments. Please review when available.

Yicong-Huang

Thanks for the effort to embed the script. however, letting python protobuf generation depends on sbt/scala/jdk is also not ideal. Can we have a better solution so that it only depends on python and a protoc lib? Sorry if this was confusing before.

Yicong-Huang · 2026-05-23T21:49:10Z

          if [ -f amber/requirements.txt ]; then uv pip install --system --index-strategy unsafe-best-match -r amber/requirements.txt; fi
          if [ -f amber/operator-requirements.txt ]; then uv pip install --system --index-strategy unsafe-best-match -r amber/operator-requirements.txt; fi
          if [ -f amber/dev-requirements.txt ]; then uv pip install --system --index-strategy unsafe-best-match -r amber/dev-requirements.txt; fi
+      - name: Install protoc 3.19.4


nit: is it possible to refer to some dependency file to declare version? so that we don't need to update CI file when need to bump protoc version.

Pinned in bin/protoc-version.txt.

Yicong-Huang · 2026-05-23T21:50:38Z

+      - name: Set up JDK 17
+        uses: actions/setup-java@v5
+        with:
+          distribution: temurin
+          java-version: 17
+      - name: Setup sbt launcher
+        uses: sbt/setup-sbt@508b753e53cb6095967669e0911487d2b9bc9f41 # v1.1.22
+      - name: Generate Python proto bindings
+        run: sbt "WorkflowExecutionService/genPythonProto"


hmm, can we not rely on sbt to generate python protobuf? it is unnecessary to install JDK just for this purpose.

The python and python-state-materialization-mac jobs now call python3 bin/gen-python-proto.py directly

Yicong-Huang · 2026-05-23T21:51:43Z

+# protoc 3.19.4 (matches PB.protocVersion in amber/build.sbt) and the
+# betterproto plugin are required by the genPythonProto sbt task so the
+# generated amber/src/main/python/proto/ tree is populated before the
+# dist is packaged.


we are hard coding this 3.19.4 version in too many places. let's define it at one place and all refer to it.

Single source of truth is bin/protoc-version.txt.

Yicong-Huang · 2026-05-23T21:52:13Z

+    && unzip -o /tmp/protoc.zip -d /usr/local \
+    && chmod +x /usr/local/bin/protoc \
+    && rm /tmp/protoc.zip \
+    && pip3 install --no-cache-dir 'betterproto[compiler]==2.0.0b7'


betterproto[compiler]==2.0.0b7 this is also hard coded. can we refer to requirements.txt instead?

Pin lives only in amber/dev-requirements.txt.

Yicong-Huang · 2026-05-23T21:52:53Z

+`amber/src/main/python/proto/` is gitignored and regenerated by the
+`genPythonProto` sbt task in [`amber/build.sbt`](amber/build.sbt), which
+runs as part of `sbt amber/compile` (or `sbt amber/genPythonProto`
+directly). Requires `protoc` on PATH (pin to `3.19.4` to match
+`PB.protocVersion`); skipped with a warning when `protoc` is missing.


let's remove this from Agent.md. too many details.

untrack generated python proto, regenerate in build

9ccc77a

github-actions Bot assigned Ma77Ball May 15, 2026

github-actions Bot added feature engine dependencies Pull requests that update a dependency file python ci changes related to CI docs Changes related to documentations dev labels May 15, 2026

github-actions Bot requested a review from Yicong-Huang May 15, 2026 11:18

Ma77Ball added 2 commits May 15, 2026 04:53

make python proto-gen robust to varying protoc install layouts

8afe6bd

make protoc include dir readable to non-root runner

ce43bfb

Yicong-Huang reviewed May 15, 2026

View reviewed changes

Comment thread amber/build.sbt Outdated

Comment thread bin/computing-unit-master.dockerfile Outdated

Yicong-Huang changed the title ~~refactor: untrack generated python proto, regenerate in build~~ [Hackathon] refactor: untrack generated python proto, regenerate in build May 15, 2026

Ma77Ball changed the title ~~[Hackathon] refactor: untrack generated python proto, regenerate in build~~ refactor: untrack generated python proto, regenerate in build May 15, 2026

Ma77Ball requested a review from Yicong-Huang May 22, 2026 08:11

Ma77Ball added 5 commits May 22, 2026 01:12

Merge branch 'main' into feat/changeProtobufGenerate

cb5b9b9

inline python proto-gen into build.sbt, drop shell script

08b3b16

Merge remote-tracking branch 'origin/feat/changeProtobufGenerate' int…

19bc840

…o feat/changeProtobufGenerate

updated comment to reference genPythonProto

d218b7d

install JDK + sbt in python jobs before genPythonProto

c83efad

Ma77Ball added 4 commits May 22, 2026 02:07

Merge branch 'main' into feat/changeProtobufGenerate

c23fb31

Merge branch 'main' into feat/changeProtobufGenerate

ec349cb

Merge branch 'main' into feat/changeProtobufGenerate

231cc39

re-run test

9c0220e

Yicong-Huang requested changes May 23, 2026

View reviewed changes

Ma77Ball added 2 commits May 23, 2026 19:49

decouple python proto gen from sbt/JDK

cb61633

Merge branch 'main' into feat/changeProtobufGenerate

32c268a

Conversation

Ma77Ball commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this PR?

Any related issues, documentation, or discussions?

How was this PR tested?

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

Uh oh!

Ma77Ball commented May 15, 2026

Uh oh!

codecov-commenter commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Yicong-Huang commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ma77Ball commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Yicong-Huang left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Ma77Ball commented May 15, 2026 •

edited

Loading

codecov-commenter commented May 15, 2026 •

edited

Loading

Yicong-Huang commented May 15, 2026 •

edited

Loading

Ma77Ball commented May 22, 2026 •

edited

Loading