fix(workflow, v1.3): a plain save no longer clobbers is_public - #8500
Merged
mengw15 merged 1 commit intoSep 11, 2026
Conversation
### What changes were proposed in this PR? Closes apache#8496. A regression from apache#8125: since then, every workflow's second autosave (and every one after it) fails with 500, silently on the operator canvas and as "Could not save" on the Form View. `persistWorkflow` wrote `is_public` from the request. The frontend feeds the saved row straight back as its metadata, and that row names the flag `isPublic` while the rest of the frontend calls it `isPublished`, so the very next save went out without the flag, the update wrote NULL into a NOT NULL column, and the request failed. A stale `isPublic: false` on a save could likewise un-publish a published workflow. - Backend: `saveWorkflowFields` now writes name, description and content only. Publishing stays with `/public` and `/private`, `default_view` with `/set-default-view`, and the timestamps are not rewritten, so a save can never clobber a concurrent change to any of them. - Frontend: `WorkflowPersistService.persistWorkflow` no longer sends `isPublic` (the endpoint does not read it, and the value is not reliably known after the first save), and `WorkflowUtilService.parseWorkflowInfo` carries a persist response's `isPublic` over to `isPublished`, so metadata fed back from a save keeps the publish state instead of dropping it. The Form View stack is not blocked by this: apache#8455 touches none of these files, and apache#8456 touches `workflow-persist.service.ts` only in `createWorkflow` (adding `defaultView`), a different function; a dry-run merge of the two is clean. ### Any related issues, documentation, discussions? Closes apache#8496. Found while verifying apache#8455 on a flag-on instance (parent apache#8011). ### How was this PR tested? Backend: `WorkflowResourceSpec` gains two tests, a save carrying no flag neither fails nor changes `is_public` after `/public`, and a save carrying `false` does not un-publish; the existing default-view save test was updated to send no flag, as the frontend does. `WorkflowResourceSpec` and `PublishedCopySchemaSpec` pass (87 tests), scalafmt clean. Frontend: the persist spec asserts the save payload carries no `isPublic` and that the response's `isPublic` comes back as `isPublished`; `parseWorkflowInfo` gains tests for the carry-over and for leaving a present `isPublished` alone. Full suite passes (5777), changed lines fully covered, eslint, prettier and the production (AOT) build pass. End to end, against a running stack rebuilt with this change: the exact second-save payload that returned 500 now returns 200 with `is_public` unchanged; a create-through-persist with the new payload inserts with `is_public = false`; and in a headless browser the Form View renames a workflow twice with every `/api/workflow/persist` answering 200, no `isPublic` key in any request body, and no "Could not save". ### Was this PR authored or co-authored using generative AI tooling? Yes. Generated-by: Claude Code (Claude Fable 5.1, Anthropic). Co-authored with Claude, reviewed line by line by the author before submission. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: Meng Wang <mengw15@uci.edu>
Contributor
Automated Reviewer SuggestionsBased on the
|
Contributor
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 364 | 0.222 | 26,373/34,696/34,696 us | 🔴 -8.6% / 🔴 +115.1% |
| 🔴 | bs=100 sw=10 sl=64 | 791 | 0.483 | 122,709/148,283/148,283 us | 🔴 +8.6% / 🔴 +36.4% |
| ⚪ | bs=1000 sw=10 sl=64 | 905 | 0.552 | 1,099,227/1,196,169/1,196,169 us | ⚪ within ±5% / 🔴 +15.3% |
Baseline details
Latest main 8391653 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 364 tuples/sec | 398 tuples/sec | 755.28 tuples/sec | -8.5% | -51.8% |
| bs=10 sw=10 sl=64 | MB/s | 0.222 MB/s | 0.243 MB/s | 0.461 MB/s | -8.6% | -51.8% |
| bs=10 sw=10 sl=64 | p50 | 26,373 us | 25,152 us | 12,957 us | +4.9% | +103.5% |
| bs=10 sw=10 sl=64 | p95 | 34,696 us | 36,626 us | 16,134 us | -5.3% | +115.1% |
| bs=10 sw=10 sl=64 | p99 | 34,696 us | 36,626 us | 20,333 us | -5.3% | +70.6% |
| bs=100 sw=10 sl=64 | throughput | 791 tuples/sec | 824 tuples/sec | 980.1 tuples/sec | -4.0% | -19.3% |
| bs=100 sw=10 sl=64 | MB/s | 0.483 MB/s | 0.503 MB/s | 0.598 MB/s | -4.0% | -19.3% |
| bs=100 sw=10 sl=64 | p50 | 122,709 us | 121,899 us | 101,894 us | +0.7% | +20.4% |
| bs=100 sw=10 sl=64 | p95 | 148,283 us | 136,538 us | 108,718 us | +8.6% | +36.4% |
| bs=100 sw=10 sl=64 | p99 | 148,283 us | 136,538 us | 122,482 us | +8.6% | +21.1% |
| bs=1000 sw=10 sl=64 | throughput | 905 tuples/sec | 903 tuples/sec | 1,011 tuples/sec | +0.2% | -10.5% |
| bs=1000 sw=10 sl=64 | MB/s | 0.552 MB/s | 0.551 MB/s | 0.617 MB/s | +0.2% | -10.6% |
| bs=1000 sw=10 sl=64 | p50 | 1,099,227 us | 1,101,175 us | 996,422 us | -0.2% | +10.3% |
| bs=1000 sw=10 sl=64 | p95 | 1,196,169 us | 1,195,549 us | 1,037,670 us | +0.1% | +15.3% |
| bs=1000 sw=10 sl=64 | p99 | 1,196,169 us | 1,195,549 us | 1,072,152 us | +0.1% | +11.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,549.44,200,128000,364,0.222,26373.04,34695.62,34695.62
1,100,10,64,20,2528.18,2000,1280000,791,0.483,122709.39,148282.64,148282.64
2,1000,10,64,20,22103.30,20000,12800000,905,0.552,1099227.17,1196168.63,1196168.63
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release/v1.3 #8500 +/- ##
===============================================
Coverage 94.07% 94.07%
Complexity 4808 4808
===============================================
Files 1194 1194
Lines 48597 48599 +2
Branches 5857 5858 +1
===============================================
+ Hits 45716 45719 +3
Misses 1428 1428
+ Partials 1453 1452 -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:
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Add coverage for the isPublic: false to isPublished: 0 conversion.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Backports the workflow-save fix so plain saves no longer overwrite publication state or fail on subsequent autosaves.
Changes:
- Restricts backend saves to editable workflow fields.
- Removes
isPublicfrom frontend save payloads. - Maps persisted
isPublicresponses toisPublished. - Adds backend and frontend regression tests.
File summaries
| File | Reviewed changes |
|---|---|
frontend/src/app/workspace/service/workflow-graph/util/workflow-util.service.ts |
Carries publication metadata into isPublished. |
frontend/src/app/workspace/service/workflow-graph/util/workflow-util.service.spec.ts |
Tests metadata conversion. |
frontend/src/app/common/service/workflow-persist/workflow-persist.service.ts |
Removes isPublic from save requests. |
frontend/src/app/common/service/workflow-persist/workflow-persist.service.spec.ts |
Verifies save payloads and response handling. |
amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala |
Tests publication-state preservation. |
amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala |
Restricts save updates to editable workflow fields. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
xuang7
approved these changes
Sep 11, 2026
mengw15
deleted the
backport/8498-a-plain-save-no-longer-clobbers-is-publi-v1.3
branch
September 11, 2026 03:24
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this PR?
Backport of #8498 to
release/v1.3: a clean cherry-pick of its squash commit, no adaptations — the backport commit reuses the squash message and author, as the automated fast path would. See #8498 for the full analysis (a regression from #8125 — every second autosave failed with 500, and a staleisPublic: falsecould un-publish a workflow).Opened manually by the v1.3 release manager: the automated post-merge direct push to release branches is currently rejected by the Merge Queue ruleset (#8377; fix in review in #8379), so this backport goes through the merge queue instead. #8498's pre-merge backport check for this branch (
apply-checkand thebackport (release/v1.3)matrix) was green.Source: 3488d37
Any related issues, documentation, discussions?
Backport of #8498. Originally linked #8496.
How was this PR tested?
The change is identical to #8498, which carries the tests (backend
WorkflowResourceSpec, frontend persist/parse specs) and an end-to-end verification; the backport tree is verified byte-identical to cherry-picking the squash commit ontorelease/v1.3, and release-branch CI runs the full matrix on this PR.Was this PR authored or co-authored using generative AI tooling?
Yes. Generated-by: Claude Code (claude-fable-5)