Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions internal/temporalcli/commands.gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -3756,9 +3756,9 @@ func NewTemporalWorkflowDeleteCommand(cctx *CommandContext, parent *TemporalWork
s.Command.Use = "delete [flags]"
s.Command.Short = "Remove Workflow Execution"
if hasHighlighting {
s.Command.Long = "Delete a Workflow Executions and its Event History:\n\n\x1b[1mtemporal workflow delete \\\n --workflow-id YourWorkflowId\x1b[0m\n\nThe removal executes asynchronously. If the Execution is Running, the Service\nterminates it before deletion.\n\nVisit https://docs.temporal.io/visibility to read more about Search Attributes\nand Query creation. See \x1b[1mtemporal batch --help\x1b[0m for a quick reference."
s.Command.Long = "Delete a Workflow Execution and its Event History:\n\n\x1b[1mtemporal workflow delete \\\n --workflow-id YourWorkflowId\x1b[0m\n\nThe removal executes asynchronously. If the Execution is Running, the Service\nterminates it before deletion.\n\nWARNING: Deleting Workflow Executions in a global Namespace removes them from\nall replicas. Requests sent to a passive cluster are forwarded to the active\ncluster by default; to target the passive cluster directly, specify\n\x1b[1m--grpc-meta xdc-redirection=false\x1b[0m.\n\nVisit https://docs.temporal.io/visibility to read more about Search Attributes\nand Query creation. See \x1b[1mtemporal batch --help\x1b[0m for a quick reference."
} else {
s.Command.Long = "Delete a Workflow Executions and its Event History:\n\n```\ntemporal workflow delete \\\n --workflow-id YourWorkflowId\n```\n\nThe removal executes asynchronously. If the Execution is Running, the Service\nterminates it before deletion.\n\nVisit https://docs.temporal.io/visibility to read more about Search Attributes\nand Query creation. See `temporal batch --help` for a quick reference."
s.Command.Long = "Delete a Workflow Execution and its Event History:\n\n```\ntemporal workflow delete \\\n --workflow-id YourWorkflowId\n```\n\nThe removal executes asynchronously. If the Execution is Running, the Service\nterminates it before deletion.\n\nWARNING: Deleting Workflow Executions in a global Namespace removes them from\nall replicas. Requests sent to a passive cluster are forwarded to the active\ncluster by default; to target the passive cluster directly, specify\n`--grpc-meta xdc-redirection=false`.\n\nVisit https://docs.temporal.io/visibility to read more about Search Attributes\nand Query creation. See `temporal batch --help` for a quick reference."
}
s.Command.Args = cobra.NoArgs
s.SingleWorkflowOrBatchOptions.BuildFlags(s.Command.Flags())
Expand Down
33 changes: 30 additions & 3 deletions internal/temporalcli/commands.workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import (

const metadataQueryName = "__temporal_workflow_metadata"

const workflowDeleteWarning = "WARNING: Deleting Workflow Executions in a global Namespace removes them from all replicas. Requests sent to a passive cluster are forwarded to the active cluster by default; to target the passive cluster directly, specify `--grpc-meta xdc-redirection=false`."

func (c *TemporalWorkflowCancelCommand) run(cctx *CommandContext, args []string) error {
cl, err := dialClient(cctx, &c.Parent.ClientOptions)
if err != nil {
Expand Down Expand Up @@ -69,13 +71,29 @@ func (c *TemporalWorkflowDeleteCommand) run(cctx *CommandContext, args []string)
}
defer cl.Close()

exec, batchReq, err := c.workflowExecOrBatch(cctx, c.Parent.Namespace, cl, singleOrBatchOverrides{})
// Only warn when the namespace is global.
nsResp, nsErr := cl.WorkflowService().DescribeNamespace(cctx, &workflowservice.DescribeNamespaceRequest{
Namespace: c.Parent.Namespace,
})
if nsErr != nil || nsResp.GetIsGlobalNamespace() {
fmt.Fprintln(cctx.Options.Stderr, workflowDeleteWarning)
}

exec, batchReq, err := c.workflowExecOrBatch(cctx, c.Parent.Namespace, cl, singleOrBatchOverrides{
AllowYesWithWorkflowID: true,
})

// Run single or batch
if err != nil {
return err
} else if exec != nil {
_, err := cl.WorkflowService().DeleteWorkflowExecution(cctx, &workflowservice.DeleteWorkflowExecutionRequest{
yes, err := cctx.promptYes(workflowDeleteSingleConfirmationMessage(exec), c.Yes)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated

if err != nil {
return err
} else if !yes {
return fmt.Errorf("user denied confirmation")
}
_, err = cl.WorkflowService().DeleteWorkflowExecution(cctx, &workflowservice.DeleteWorkflowExecutionRequest{
Namespace: c.Parent.Namespace,
WorkflowExecution: &common.WorkflowExecution{WorkflowId: c.WorkflowId, RunId: c.RunId},
})
Expand All @@ -96,6 +114,14 @@ func (c *TemporalWorkflowDeleteCommand) run(cctx *CommandContext, args []string)
return nil
}

func workflowDeleteSingleConfirmationMessage(exec *common.WorkflowExecution) string {
action := fmt.Sprintf("Delete Workflow %q", exec.GetWorkflowId())
if exec.GetRunId() != "" {
action += fmt.Sprintf(" with Run ID %q", exec.GetRunId())
}
return fmt.Sprintf("%s? y/N", action)
}

func (c *TemporalWorkflowUpdateOptionsCommand) run(cctx *CommandContext, args []string) error {
cl, err := dialClient(cctx, &c.Parent.ClientOptions)
if err != nil {
Expand Down Expand Up @@ -511,6 +537,7 @@ func defaultReason() string {

type singleOrBatchOverrides struct {
AllowReasonWithWorkflowID bool
AllowYesWithWorkflowID bool
}

func (s *SingleWorkflowOrBatchOptions) workflowExecOrBatch(
Expand All @@ -525,7 +552,7 @@ func (s *SingleWorkflowOrBatchOptions) workflowExecOrBatch(
return nil, nil, fmt.Errorf("cannot set query when workflow ID is set")
} else if s.Reason != "" && !overrides.AllowReasonWithWorkflowID {
return nil, nil, fmt.Errorf("cannot set reason when workflow ID is set")
} else if s.Yes {
} else if s.Yes && !overrides.AllowYesWithWorkflowID {
return nil, nil, fmt.Errorf("cannot set 'yes' when workflow ID is set")
} else if s.Rps != 0 {
return nil, nil, fmt.Errorf("cannot set rps when workflow ID is set")
Expand Down
41 changes: 41 additions & 0 deletions internal/temporalcli/commands.workflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,44 @@ func (s *SharedServerSuite) testSignalBatchWorkflow(json bool) *CommandResult {
return res
}

func (s *SharedServerSuite) TestWorkflow_Delete_SingleWorkflowRequiresConfirmation() {
res := s.Execute(
"workflow", "delete",
"--address", s.Address(),
"--workflow-id", "delete-confirmation-test",
)
s.EqualError(res.Err, "user denied confirmation")
// Dev-server's default namespace is non-global, so the warning is suppressed.
s.NotContains(res.Stdout.String(), "Deleting Workflow Executions in a global Namespace")
s.NotContains(res.Stderr.String(), "Deleting Workflow Executions in a global Namespace")
}

func (s *SharedServerSuite) TestWorkflow_Delete_SingleWorkflowSuccess() {
s.Worker().OnDevWorkflow(func(ctx workflow.Context, a any) (any, error) {
ctx.Done().Receive(ctx, nil)
return nil, ctx.Err()
})

run, err := s.Client.ExecuteWorkflow(
s.Context,
client.StartWorkflowOptions{TaskQueue: s.Worker().Options.TaskQueue},
DevWorkflow,
"ignored",
)
s.NoError(err)

res := s.Execute(
"workflow", "delete",
"--address", s.Address(),
"--workflow-id", run.GetID(),
"--yes",
)
s.NoError(res.Err)
s.NotContains(res.Stdout.String(), "Deleting Workflow Executions in a global Namespace")
s.NotContains(res.Stderr.String(), "Deleting Workflow Executions in a global Namespace")
s.Contains(res.Stdout.String(), "Delete workflow succeeded")
}

func (s *SharedServerSuite) TestWorkflow_Delete_BatchWorkflowSuccess() {
s.Worker().OnDevWorkflow(func(ctx workflow.Context, a any) (any, error) {
ctx.Done().Receive(ctx, nil)
Expand Down Expand Up @@ -212,6 +250,9 @@ func (s *SharedServerSuite) TestWorkflow_Delete_BatchWorkflowSuccess() {
"-y",
)
s.NoError(res.Err)
s.Contains(res.Stdout.String(), "Start batch against approximately 2 workflow(s)")
s.NotContains(res.Stdout.String(), "Deleting Workflow Executions in a global Namespace")
s.NotContains(res.Stderr.String(), "Deleting Workflow Executions in a global Namespace")

// Confirm workflows were deleted
s.Eventually(func() bool {
Expand Down
7 changes: 6 additions & 1 deletion internal/temporalcli/commands.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3665,7 +3665,7 @@ commands:
- name: temporal workflow delete
summary: Remove Workflow Execution
description: |
Delete a Workflow Executions and its Event History:
Delete a Workflow Execution and its Event History:

```
temporal workflow delete \
Expand All @@ -3675,6 +3675,11 @@ commands:
The removal executes asynchronously. If the Execution is Running, the Service
terminates it before deletion.

WARNING: Deleting Workflow Executions in a global Namespace removes them from
all replicas. Requests sent to a passive cluster are forwarded to the active
cluster by default; to target the passive cluster directly, specify
`--grpc-meta xdc-redirection=false`.

Visit https://docs.temporal.io/visibility to read more about Search Attributes
and Query creation. See `temporal batch --help` for a quick reference.
option-sets:
Expand Down
Loading