💥 Workflow delete now prompts for confirmation#1029
Conversation
|
|
|
|
| return err | ||
| } else if exec != nil { | ||
| _, err := cl.WorkflowService().DeleteWorkflowExecution(cctx, &workflowservice.DeleteWorkflowExecutionRequest{ | ||
| yes, err := cctx.promptYes(workflowDeleteSingleConfirmationMessage(exec), c.Yes) |
There was a problem hiding this comment.
Prompting would be a breaking change if used in a script. Can you follow this in the PR title/description?
Breaking changes must be treated with high review rigor, marked with 💥 (:boom:) in commit messages and the PR title/description, and called out with 💥 in the release notes with a clear explanation.
FYI we're in process of standardizing this
| defer cl.Close() | ||
|
|
||
| exec, batchReq, err := c.workflowExecOrBatch(cctx, c.Parent.Namespace, cl, singleOrBatchOverrides{}) | ||
| cctx.Printer.Println(workflowDeleteWarning) |
There was a problem hiding this comment.
This will print for all namespace types including non-global ones. I'd suggest either
- Making the message more specific, i.e. "WARNING: If this Namespace is global, deleting Workflow Executions in the namespace removes them ..."
- Or better UX at the cost of extra RPC: Call DescribeNamespace, check GetIsGlobalNamespace and print warning conditionally
There was a problem hiding this comment.
+1 on this, would it be possible to show this warning only for replicated users in a easier way? Users running a single cluster might be confused with this warning.
There was a problem hiding this comment.
Good point! Updated to check on global namespace.
| return nil | ||
| } | ||
|
|
||
| func workflowDeleteConfirmationMessage(action string) string { |
| defer cl.Close() | ||
|
|
||
| exec, batchReq, err := c.workflowExecOrBatch(cctx, c.Parent.Namespace, cl, singleOrBatchOverrides{}) | ||
| cctx.Printer.Println(workflowDeleteWarning) |
There was a problem hiding this comment.
Currently the warning goes to stdout, so --output json callers get a non-JSON line with their output. Send to stderr for warnings:
| cctx.Printer.Println(workflowDeleteWarning) | |
| fmt.Fprintln(cctx.Options.Stderr, workflowDeleteWarning) |
Single-workflow `temporal workflow delete` now requires interactive confirmation (or `--yes`/`-y`). Scripts that previously relied on the command running non-interactively will break unless they pass `--yes`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jiechenz
changed the title
jiechenz
changed the title
jiechenz
changed the title
6 participants
What was changed
Added a warning to
temporal workflow deleteexplaining that deleting Workflow Executions in aglobal Namespace removes them from all replicas, and that requests sent to a passive cluster are forwarded to the active cluster by default unless
--grpc-meta xdc-redirection=falseis specified.Delete workflow now prompts for confirmation
Why?
The CLI should make that blast radius clear before users confirm deletion.
The passive-cluster note helps users who intentionally want to target a passive cluster avoid the default frontend forwarding behavior.
Checklist
Closes
How was this tested:
unit-test and local run.
single deletion
batch deletion