UX-179 - rpcn: save pipelines as drafts - #2624
Conversation
Save used to mean Start, and was gated on validation passing. So work in
progress could not be parked — an invalid config would not save at all, and a
refresh or a misclicked Back button lost everything typed — and a finished
config could not be saved without going live.
A pipeline can now be saved as a draft: persisted, not running, zero compute,
and stored exactly as typed even when it does not lint. Validation moves to the
moment it matters, which is Start.
API (proto/redpanda/api/dataplane/v1/pipeline.proto), all additive:
- Pipeline.State.STATE_DRAFT, omitted from ListPipelines unless
Filter.include_drafts is set, so clients written before drafts existed never
receive a state they cannot render.
- PipelineCreate.draft, and PipelineUpdate.draft as an *assertion* rather than
a transition: it means "I am editing a draft", and the update fails with
FAILED_PRECONDITION if the pipeline has since been started. Without that,
"Save draft" could silently deploy a config to a running pipeline because a
teammate started it mid-edit.
- Filter.states for state filtering, and Pipeline.created_by / create_time /
update_time so a shared draft pool is attributable and its staleness visible.
- config_yaml loses its field-level `required` rule; the service enforces it
for anything that is not a draft, with a sentence instead of a proto field
path. Pipeline keeps the invariant as a message-level CEL rule.
Console:
- Split save actions per context: Save draft (new pipelines and drafts),
Save and start, Save (stopped), Apply and restart (running — saying "Save"
would hide the restart, and there is no apply-later to make that untrue).
- Drafts are ordinary rows in the pipeline list: Draft chip, Drafts tab with a
count, sorted first and by last edited, resume / start / delete actions, and
"Edited 5m ago · by someone" in place of the id.
- A draft's own page explains itself instead of offering monitoring it cannot
have, and starting one routes lint failures into the editor where they are
fixable.
- Lint results are warnings on a draft and block only Start; an unnamed draft
is auto-named rather than refused.
- localStorage is repurposed from "drafts" to what it is actually good for:
crash recovery for the editor buffer, offered back after a refresh and
dropped on a successful save.
- New Changes lane (YAML | Visual | Changes N) diffing the saved config
against the editor, with the components touched listed and clickable, and a
header line stating what applying will cost.
Deliberate decisions, with reasoning, in frontend/specs/rp-connect-pipeline-drafts.md:
draft as a distinct state rather than an unapplied-changes flag; annotation
storage rather than a CRD field; drafts count against the pipeline quota; drafts
are named and names may collide; org-wide visibility because pipeline RBAC has no
ownership predicate; no expiry, staleness shown instead. It also records the
feature's real limit — a draft ends at first start, so "save without going live"
is solved for pipelines that have never run and not for the ones that have — and
the design for closing that with a pending revision.
Ships behind enable-rpcn-pipeline-drafts. The flag must stay off until the
redpanda-connect-api carrying draft support is deployed: an older API ignores
`draft` on create and deploys what the user asked to park. Note that
backend/pkg/protogen must be regenerated even though no Go here implements
PipelineService — console-enterprise proxies through these types, and Connect's
JSON codec discards unknown fields rather than passing them through.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Autosave restore resets the form without keepDirty, so a settings-only restore arms the unsaved-changes guard (regression test added). - Regenerate proto/gen/openapi for the pipeline proto changes; CI dirty-checks it alongside the protogen dirs. - Spec: deploy order is free while the flag is off; only the flag flip is ordered. - Trim comments to the constraint, not the rationale. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
The latest Buf updates on your PR. Results from workflow Buf CI / validate (pull_request).
|
✅ Clean — no registry drift, off-token colours, or ad-hoc classesApp:
Generated by lookout audit-changes. |
SpicyPete
changed the title
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Also lands under frontend/**, which the verify and dispatch workflows filter on; the previous empty commit ran none of them. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
SpicyPete
changed the title
| // FAILED_PRECONDITION if the pipeline has since been started. An update never | ||
| // changes whether a pipeline is a draft; false is rejected, and StartPipeline | ||
| // promotes a draft. Leave unset to update whatever is stored. | ||
| optional bool draft = 9; |
There was a problem hiding this comment.
can you have a draft that's not technically valid YAML so that you can leave it and fix it before it can move to a non-draft state?
There was a problem hiding this comment.
Yes, you can save invalid YAML as a draft
…rafts `draft` is a plain proto3 bool, so a pre-drafts dataplane drops it in transit and CreatePipeline deploys for real. The editor already noticed (it trusts the response state, not the flag it sent) but only said so in a toast, leaving an unwanted pipeline consuming from its inputs and writing to its outputs until a human read the message and acted. The window is real on every cluster upgrade: `console_image_tag` and `redpanda_connect_api_version` ride the same install pack but roll as separate deployments. Issue the follow-up stop instead — the same one `run: 'stopped'` already does for a create on a pre-drafts dataplane — and keep the old copy for when that stop itself fails. The stop does not undo what already ran, so the copy says so. ListPipelines now asks for drafts whether or not the feature is enabled. The flag gates making drafts, not seeing the ones already made, and turning it back off stranded them: excluded from the list, so no Drafts tab and no row to delete from, while each still counted against the cluster's pipeline quota. Every draft affordance in the row keys off the row's own state, and a pre-drafts dataplane drops the field, so the request is inert until there is a draft to show. Also guard the list's status-badge variant lookup, as the label beside it and the detail page's badge already do: an unrecognised state fell through to StatusBadge's `informative` default next to an "Unknown" label. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
nit: noticed this while testing and wanted to log it somewhere...
|
|
nit: "new" badge in header is visual noise. suggest removing when status is "new". the status indicator on the right side (unsaved changes) is enough for me to know the state 
|
|
minor: when pressing save button dropdown the border radii disappear. using ui-registry group with 2 registry buttons as children should fix that |
|
minor: I feel like the "start pipeline" button on the subsequent page + success toast that indicates the "pipeline was saved but not started" does the job for us. minor: Can we make the success toast that shows on saving a pipeline last a little longer? 1s duration was too fast for me to read it |
|
major: noticed another bug while testing where pipeline starts, can see on pipeline detail page, but pipelines list page still shows as "starting" even after refreshing |
|
nit: can we hide or disable the unsaved changes tab when none exist? |
|
nit: after testing many states, I feel like the helper text below the save button isn't useful at all, consider omitting? Seems like a claude addition to account for killing the previous "start pipeline on save behavior". |
Fixed all of the issues I think, however this one I am not able to reproduce locally. Let me know if you're still seeing it on the next review. |
| // Every field `applyAutosave` hands to `form.reset`, so a stale shape can't crash the editor. | ||
| function isAutosaveEntry(value: unknown): value is EditorAutosaveEntry { | ||
| if (value === null || typeof value !== 'object') { | ||
| return false; | ||
| } | ||
| const entry = value as Partial<EditorAutosaveEntry>; | ||
| return ( | ||
| entry.version === AUTOSAVE_ENTRY_VERSION && | ||
| typeof entry.targetKey === 'string' && | ||
| typeof entry.clusterId === 'string' && | ||
| typeof entry.name === 'string' && | ||
| typeof entry.description === 'string' && | ||
| typeof entry.computeUnits === 'number' && | ||
| Array.isArray(entry.tags) && | ||
| entry.tags.every(isAutosaveTag) && | ||
| typeof entry.configYaml === 'string' && | ||
| typeof entry.updatedAt === 'number' && | ||
| (entry.basedOnUpdateTime === undefined || | ||
| entry.basedOnUpdateTime === null || | ||
| typeof entry.basedOnUpdateTime === 'number') | ||
| ); | ||
| } |
There was a problem hiding this comment.
there must be a better way to validate than this? seems overly complex
| // FAILED_PRECONDITION if the pipeline has since been started. An update never | ||
| // changes whether a pipeline is a draft; false is rejected, and StartPipeline | ||
| // promotes a draft. Leave unset to update whatever is stored. | ||
| optional bool draft = 9; |
There was a problem hiding this comment.
[ultra nit]: I would have expected something like is_draft to make it clear it's a boolean
malinskibeniamin
left a comment
malinskibeniamin
left a comment
There was a problem hiding this comment.
I think someone else should be asked to review backend parts
|
Backend half of this has already merged, will address comments, thank you |
| const markSaved = useCallback( | ||
| (yamlContent: string, savedPipelineId: string | undefined, savedUpdateTime: number | null) => { | ||
| editorStore.getState().markSavedBaseline(yamlContent, savedUpdateTime); | ||
| form.reset(form.getValues()); |
There was a problem hiding this comment.
[P1] Rebaseline settings to the submitted snapshot, not current values
The settings remain editable while a save is pending. If a user saves an existing draft with name A, then types name B before UpdatePipeline resolves, this reset marks B as the saved default even though the request only contained A. The draft editor stays open, the unsaved-changes guard returns false, and the recovery buffer is cleared, so leaving loses B without warning. This also affects description, compute units, and tags.
Reproduced with the actual PipelinePage and a deferred UpdatePipeline transport: the captured request contains submitted-name, the input still displays newer-unsaved-name, but shouldBlockFn() returns false after success. Preserve a snapshot of the submitted settings as the baseline and retain subsequent edits as dirty; do not clear their recovery buffer. Alternatively, prevent settings edits for the entire save operation.
Verify: delay a draft update response, change a setting after submitting, then complete the response. The newer setting must remain unsaved and recoverable.
| // skips the stop entirely, and the copy below must not then claim it was parked. | ||
| let stopFailed = false; | ||
| let stopped = false; | ||
| if ((run === 'stopped' || draftWasIgnored) && newPipelineId) { |
There was a problem hiding this comment.
[P1] Do not implement a non-starting save by deploying and then stopping
With the drafts flag off (the default), the primary Save action selects run: 'stopped', but the preceding CreatePipeline call sends draft: false. The API contract explicitly starts that pipeline; this separate StopPipeline request can only happen afterward (and after the create hook's awaited query invalidations). A fast input can process messages or write externally before the stop arrives, even when both requests succeed. The leave dialog nevertheless promises “without starting it,” and the success toast says “it is not running yet.”
The existing flag-off test confirms the deploy-then-stop request sequence, rather than proving a non-starting save. Only offer that promise when the service supports an atomic non-running creation path. Otherwise keep creation explicitly labeled as deployment/start, with matching confirmation and copy, instead of presenting this as a safe parked save.
Verify: with the flag off and the stop response delayed, the primary non-starting Save must not issue a request that starts data processing.
3 participants
Summary
Save no longer means deploy. A pipeline can be saved as a draft: stored as written, never validated, never running, and started later once it lints. Around that the editor gets crash recovery and a way to see what a save will change.
dataplane/v1/pipeline.proto, additive):STATE_DRAFT,PipelineCreate.draft,PipelineUpdate.draft, list filtersstates/include_drafts, and output-onlycreated_by,create_time,update_time. Drafts stay out ofListPipelinesunless asked for, so older clients never see a state they can't render.localStorage, so a crash or a closed tab doesn't lose work. The next visit offers to restore them and warns if someone saved the pipeline in the meantime.The reasoning behind the design choices is in
frontend/specs/rp-connect-pipeline-drafts.md.Rollout
Behind
enable-rpcn-pipeline-drafts, off by default here and in cloud-ui. The service side is redpanda-data/cloudv2#29544.Deploy order doesn't matter while the flag is off. Once it's on, every hop (console, console-enterprise, redpanda-connect-api) has to be new: an old one drops
draftand deploys the pipeline for real. The UI checks the state that comes back and tells the user if that happened. Flip the flag last.backend/pkg/protogenandproto/gen/openapiare regenerated because console-enterprise proxies this service through the Go types and would otherwise strip the new fields.