Skip to content

fix(workflow): a plain save no longer clobbers is_public - #8498

Merged
mengw15 merged 2 commits into
apache:mainfrom
yangzhang75:fix/persist-is-public
Sep 11, 2026
Merged

fix(workflow): a plain save no longer clobbers is_public#8498
mengw15 merged 2 commits into
apache:mainfrom
yangzhang75:fix/persist-is-public

Conversation

@yangzhang75

@yangzhang75 yangzhang75 commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Closes #8496. A regression from #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: #8455 touches none of these files, and #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 #8496. Found while verifying #8455 on a flag-on instance (parent #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.ai/code/session_01FVvP3ttj22f9LB4p9u2anY

Since apache#8125 the persist endpoint wrote is_public from the request. The frontend feeds the saved
row back as its metadata, and that row names the flag isPublic while the rest of the frontend
calls it isPublished, so the very next autosave went out without the flag, the update wrote NULL
into a NOT NULL column, and every second save failed with 500 (silently on the canvas, "Could not
save" on the Form View). A stale false on a save could likewise un-publish a workflow.

persist now writes name, description and content only; publishing stays with /public and
/private, and default_view with /set-default-view. The frontend stops sending the flag on a save,
and parseWorkflowInfo carries a persist response's isPublic over to isPublished so metadata fed
back from a save keeps the publish state.

Closes apache#8496.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY
@github-actions github-actions Bot added engine fix frontend Changes related to the frontend GUI labels Sep 11, 2026
@Yicong-Huang Yicong-Huang added release/v1.3 back porting to release/v1.3 release/v1.2 back porting to release/v1.2 labels Sep 11, 2026
@github-actions
github-actions Bot requested review from mengw15 and xuang7 September 11, 2026 01:47
@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Backport auto-label report

This fix: PR was checked against each actively-supported release branch. A release/* label nominates a backport target; the branch's release manager approving this PR is what sends the fix there. The required Backport Approvals check stays red until every label below is approved, so each manager either approves or removes their own label — which is why the labels left on a merged PR are exactly the branches it reached.

Release branch Analysis
release/v1.3 Already labeled — this fix is queued to backport here. @mengw15 decides: approving sends the fix here, removing this label declines it. The merge waits on one or the other.
🚫 release/v1.2 Label was removed earlier (opt-out); not re-added. Re-add it by hand if this fix should be backported here after all — the branch's release manager then has to approve before this PR can merge.

Auto-label run.

@github-actions

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: @aglinxinyuan
    You can notify them by mentioning @aglinxinyuan in a comment.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The backend and frontend changes consistently address the regression with focused coverage for the affected save paths.

Pull request overview

Fixes workflow autosave failures and prevents ordinary saves from overwriting publication state.

Changes:

  • Restricts backend saves to name, description, and content.
  • Normalizes persisted isPublic responses to frontend isPublished.
  • Adds regression coverage for missing and stale publication flags.
File summaries
File Description
WorkflowResource.scala Preserves publication state during saves.
WorkflowResourceSpec.scala Adds backend regression tests.
workflow-persist.service.ts Omits publication state from save requests.
workflow-persist.service.spec.ts Verifies request and response behavior.
workflow-util.service.ts Normalizes the publication field name.
workflow-util.service.spec.ts Tests publication metadata normalization.
Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

🟢 2 better · 🔴 8 worse · ⚪ 5 noise (<±5%) · 0 without baseline

Compared against main 0e549fa benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

config throughput MB/s latency max Δ latest / 7d
🔴 bs=10 sw=10 sl=64 377 0.23 26,406/31,863/31,863 us 🟢 -32.7% / 🔴 +103.8%
🔴 bs=100 sw=10 sl=64 781 0.476 124,770/162,311/162,311 us 🔴 +20.2% / 🔴 +49.3%
bs=1000 sw=10 sl=64 919 0.561 1,086,997/1,157,494/1,157,494 us ⚪ within ±5% / 🔴 +11.5%
Baseline details

Latest main 0e549fa from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 377 tuples/sec 415 tuples/sec 755.28 tuples/sec -9.2% -50.1%
bs=10 sw=10 sl=64 MB/s 0.23 MB/s 0.253 MB/s 0.461 MB/s -9.1% -50.1%
bs=10 sw=10 sl=64 p50 26,406 us 20,222 us 12,957 us +30.6% +103.8%
bs=10 sw=10 sl=64 p95 31,863 us 47,368 us 16,134 us -32.7% +97.5%
bs=10 sw=10 sl=64 p99 31,863 us 47,368 us 20,333 us -32.7% +56.7%
bs=100 sw=10 sl=64 throughput 781 tuples/sec 850 tuples/sec 980.1 tuples/sec -8.1% -20.3%
bs=100 sw=10 sl=64 MB/s 0.476 MB/s 0.519 MB/s 0.598 MB/s -8.3% -20.4%
bs=100 sw=10 sl=64 p50 124,770 us 115,658 us 101,894 us +7.9% +22.5%
bs=100 sw=10 sl=64 p95 162,311 us 135,032 us 108,718 us +20.2% +49.3%
bs=100 sw=10 sl=64 p99 162,311 us 135,032 us 122,482 us +20.2% +32.5%
bs=1000 sw=10 sl=64 throughput 919 tuples/sec 929 tuples/sec 1,011 tuples/sec -1.1% -9.1%
bs=1000 sw=10 sl=64 MB/s 0.561 MB/s 0.567 MB/s 0.617 MB/s -1.1% -9.1%
bs=1000 sw=10 sl=64 p50 1,086,997 us 1,068,865 us 996,422 us +1.7% +9.1%
bs=1000 sw=10 sl=64 p95 1,157,494 us 1,193,661 us 1,037,670 us -3.0% +11.5%
bs=1000 sw=10 sl=64 p99 1,157,494 us 1,193,661 us 1,072,152 us -3.0% +8.0%
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,530.08,200,128000,377,0.230,26406.34,31863.05,31863.05
1,100,10,64,20,2561.98,2000,1280000,781,0.476,124769.78,162311.19,162311.19
2,1000,10,64,20,21770.03,20000,12800000,919,0.561,1086997.12,1157494.01,1157494.01

@codecov-commenter

codecov-commenter commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.90%. Comparing base (0e549fa) to head (f8bebf8).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8498      +/-   ##
============================================
+ Coverage     93.69%   93.90%   +0.21%     
- Complexity     4825     4944     +119     
============================================
  Files          1209     1213       +4     
  Lines         49871    53030    +3159     
  Branches       6099     6804     +705     
============================================
+ Hits          46726    49800    +3074     
- Misses         1652     1705      +53     
- Partials       1493     1525      +32     
Flag Coverage Δ *Carryforward flag
access-control-service 80.18% <ø> (ø) Carriedforward from 1fbd346
agent-service 99.32% <ø> (ø) Carriedforward from 1fbd346
amber 90.20% <100.00%> (+0.29%) ⬆️
computing-unit-managing-service 77.14% <ø> (ø) Carriedforward from 1fbd346
config-service 87.12% <ø> (ø) Carriedforward from 1fbd346
file-service 83.65% <ø> (ø) Carriedforward from 1fbd346
frontend 96.31% <100.00%> (+0.14%) ⬆️
notebook-migration-service 83.73% <ø> (ø) Carriedforward from 1fbd346
pyamber 98.47% <ø> (ø) Carriedforward from 1fbd346
workflow-compiling-service 77.19% <ø> (ø) Carriedforward from 1fbd346

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

@mengw15 mengw15 removed the release/v1.2 back porting to release/v1.2 label Sep 11, 2026

@mengw15 mengw15 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mengw15
mengw15 enabled auto-merge September 11, 2026 02:18
@mengw15
mengw15 added this pull request to the merge queue Sep 11, 2026
Merged via the queue into apache:main with commit 3488d37 Sep 11, 2026
41 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

Backport to release/v1.3 failed. See job log.

@mengw15

mengw15 commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Backported to release/v1.3 via #8500.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

engine fix frontend Changes related to the frontend GUI release/v1.3 back porting to release/v1.3

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(workflow): a plain save writes is_public as NULL from the second autosave on (regression from #8125)

5 participants