-
Notifications
You must be signed in to change notification settings - Fork 100
Add batch cancel, terminate and delete operators for standalone activities #1100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
f74d1ce
a01b1e0
8125e07
cad24b5
1bca399
5e26355
c522e9d
102faf5
ec933c0
b1340a3
a6a4dd6
a41ccee
1cf0210
478503a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,15 @@ | ||
| package temporalcli | ||
|
|
||
| import ( | ||
| "cmp" | ||
| "context" | ||
| "encoding/json" | ||
| "errors" | ||
| "fmt" | ||
| "time" | ||
|
|
||
| "github.com/fatih/color" | ||
| "github.com/google/uuid" | ||
| "github.com/temporalio/cli/internal/printer" | ||
| activitypb "go.temporal.io/api/activity/v1" | ||
| "go.temporal.io/api/batch/v1" | ||
|
|
@@ -40,6 +42,8 @@ type ( | |
| } | ||
| ) | ||
|
|
||
| const activityDeleteWarning = "WARNING: Deleting Standalone Activity 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 *TemporalActivityStartCommand) run(cctx *CommandContext, args []string) error { | ||
| cl, err := dialClient(cctx, &c.Parent.ClientOptions) | ||
| if err != nil { | ||
|
|
@@ -562,21 +566,107 @@ func (c *TemporalActivityCountCommand) run(cctx *CommandContext, args []string) | |
| return nil | ||
| } | ||
|
|
||
| func (s *ActivityReferenceOrBatchOptions) activityExecOrBatch( | ||
| cctx *CommandContext, | ||
| namespace string, | ||
| cl client.Client, | ||
| yesFlag bool, | ||
| overrides singleOrBatchOverrides, | ||
| ) (*client.GetActivityHandleOptions, *workflowservice.StartBatchOperationRequest, error) { | ||
| // If activity is set, we return activity handle options with activity ID and run ID | ||
| if s.ActivityId != "" { | ||
| if s.Query != "" { | ||
| return nil, nil, fmt.Errorf("cannot set query when activity ID is set") | ||
| } else if yesFlag && !overrides.AllowYesWithActivityID { | ||
| return nil, nil, fmt.Errorf("cannot set 'yes' when activity ID is set") | ||
| } else if s.Rps != 0 { | ||
| return nil, nil, fmt.Errorf("cannot set rps when activity ID is set") | ||
| } | ||
| return &client.GetActivityHandleOptions{ | ||
| ActivityID: s.ActivityId, | ||
| RunID: s.RunId, | ||
| }, nil, nil | ||
| } | ||
|
|
||
| // Check query is set properly | ||
| if s.Query == "" { | ||
| return nil, nil, fmt.Errorf("must set either activity ID or query") | ||
| } else if s.ActivityId != "" { // This is redundant, but kept for completeness | ||
| return nil, nil, fmt.Errorf("cannot set activity ID when query is set") | ||
| } else if s.RunId != "" { | ||
| return nil, nil, fmt.Errorf("cannot set run ID when query is set") | ||
| } | ||
|
|
||
| // The count is only used in the confirmation prompt; skip the request when --yes | ||
| // bypasses it, so batch jobs can still proceed if the visibility API is timing out. | ||
| var promptMessage string | ||
| if yesFlag { | ||
| promptMessage = fmt.Sprintf("Start batch against standalone activities matching query %q? y/N", s.Query) | ||
| } else { | ||
| count, err := cl.CountActivities(cctx, client.CountActivitiesOptions{Query: s.Query}) | ||
| if err != nil { | ||
| return nil, nil, fmt.Errorf("failed counting standalone activities from query: %w", err) | ||
| } | ||
| promptMessage = fmt.Sprintf("Start batch against approximately %v standalone activities(s)? y/N", count.Count) | ||
| } | ||
| isYes, err := cctx.promptYes(promptMessage, yesFlag) | ||
| if err != nil { | ||
| return nil, nil, err | ||
| } else if !isYes { | ||
| // We consider this a command failure | ||
| return nil, nil, fmt.Errorf("user denied confirmation") | ||
| } | ||
|
|
||
| return nil, &workflowservice.StartBatchOperationRequest{ | ||
| MaxOperationsPerSecond: s.Rps, | ||
| Namespace: namespace, | ||
| JobId: uuid.NewString(), | ||
| VisibilityQuery: s.Query, | ||
| }, nil | ||
| } | ||
|
|
||
| func (c *TemporalActivityCancelCommand) run(cctx *CommandContext, args []string) error { | ||
| cl, err := dialClient(cctx, &c.Parent.ClientOptions) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| defer cl.Close() | ||
|
|
||
| handle := cl.GetActivityHandle(client.GetActivityHandleOptions{ | ||
| ActivityID: c.ActivityId, | ||
| RunID: c.RunId, | ||
| }) | ||
| if err := handle.Cancel(cctx, client.CancelActivityOptions{Reason: c.Reason}); err != nil { | ||
| return fmt.Errorf("failed to request activity cancellation: %w", err) | ||
| opts := ActivityReferenceOrBatchOptions{ | ||
| ActivityId: c.ActivityId, | ||
| RunId: c.RunId, | ||
| Query: c.Query, | ||
| Rps: c.Rps, | ||
| } | ||
|
|
||
| activityOptions, batchReq, err := opts.activityExecOrBatch(cctx, c.Parent.Namespace, cl, c.Yes, singleOrBatchOverrides{}) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if activityOptions != nil { | ||
| handle := cl.GetActivityHandle(*activityOptions) | ||
| if err := handle.Cancel(cctx, client.CancelActivityOptions{Reason: c.Reason}); err != nil { | ||
| return fmt.Errorf("failed to request activity cancellation: %w", err) | ||
| } | ||
| cctx.Printer.Println("Cancellation requested") | ||
| } else { // batchReq != nil | ||
| cancelActivitiesOperation := &batch.BatchOperationCancelActivities{ | ||
| Identity: c.Parent.Identity, | ||
| // do not fallback to defaultReason, to be consistent with single activity cancel | ||
| Reason: c.Reason, | ||
| } | ||
|
|
||
| // Reason in batch request falls back to defaultReason | ||
| batchReq.Reason = cmp.Or(c.Reason, defaultReason()) | ||
| batchReq.Operation = &workflowservice.StartBatchOperationRequest_CancelActivitiesOperation{ | ||
| CancelActivitiesOperation: cancelActivitiesOperation, | ||
| } | ||
|
|
||
| if err := startBatchJob(cctx, cl, batchReq); err != nil { | ||
| return err | ||
| } | ||
| } | ||
| cctx.Printer.Println("Cancellation requested") | ||
| return nil | ||
| } | ||
|
|
||
|
|
@@ -587,24 +677,117 @@ func (c *TemporalActivityTerminateCommand) run(cctx *CommandContext, args []stri | |
| } | ||
| defer cl.Close() | ||
|
|
||
| // The CLI adds a default for terminate but not cancel. | ||
| // This matches the behavior for workflows. | ||
| reason := c.Reason | ||
| if reason == "" { | ||
| reason = defaultReason() | ||
| opts := ActivityReferenceOrBatchOptions{ | ||
| ActivityId: c.ActivityId, | ||
| RunId: c.RunId, | ||
| Query: c.Query, | ||
| Rps: c.Rps, | ||
| } | ||
| handle := cl.GetActivityHandle(client.GetActivityHandleOptions{ | ||
| ActivityID: c.ActivityId, | ||
| RunID: c.RunId, | ||
|
|
||
| activityOptions, batchReq, err := opts.activityExecOrBatch(cctx, c.Parent.Namespace, cl, c.Yes, singleOrBatchOverrides{}) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // Reason for single terminate or batch request falls back to defaultReason | ||
| reason := cmp.Or(c.Reason, defaultReason()) | ||
|
|
||
| if activityOptions != nil { | ||
| // The CLI adds a default for terminate but not cancel. | ||
| // This matches the behavior for workflows. | ||
| handle := cl.GetActivityHandle(*activityOptions) | ||
| // Terminate may fail if the activity doesn't exist or has already completed. | ||
| if err := handle.Terminate(cctx, client.TerminateActivityOptions{Reason: reason}); err != nil { | ||
| return fmt.Errorf("failed to terminate activity: %w", err) | ||
| } | ||
| cctx.Printer.Println("Activity terminated") | ||
| } else { // batchReq != nil | ||
| terminateActivitiesOperation := &batch.BatchOperationTerminateActivities{ | ||
| Identity: c.Parent.Identity, | ||
| Reason: reason, | ||
| } | ||
|
|
||
| batchReq.Reason = reason | ||
| batchReq.Operation = &workflowservice.StartBatchOperationRequest_TerminateActivitiesOperation{ | ||
| TerminateActivitiesOperation: terminateActivitiesOperation, | ||
| } | ||
|
|
||
| if err := startBatchJob(cctx, cl, batchReq); err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (c *TemporalActivityDeleteCommand) run(cctx *CommandContext, args []string) error { | ||
| cl, err := dialClient(cctx, &c.Parent.ClientOptions) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| defer cl.Close() | ||
|
|
||
| // Only warn when the namespace is global, or can't get the namespace info | ||
| nsResp, nsErr := cl.WorkflowService().DescribeNamespace(cctx, &workflowservice.DescribeNamespaceRequest{ | ||
| Namespace: c.Parent.Namespace, | ||
| }) | ||
| // Terminate may fail if the activity doesn't exist or has already completed. | ||
| if err := handle.Terminate(cctx, client.TerminateActivityOptions{Reason: reason}); err != nil { | ||
| return fmt.Errorf("failed to terminate activity: %w", err) | ||
| if nsErr != nil || nsResp.GetIsGlobalNamespace() { | ||
| fmt.Fprintln(cctx.Options.Stderr, activityDeleteWarning) | ||
| } | ||
| cctx.Printer.Println("Activity terminated") | ||
|
|
||
| opts := ActivityReferenceOrBatchOptions{ | ||
| ActivityId: c.ActivityId, | ||
| RunId: c.RunId, | ||
| Query: c.Query, | ||
| Rps: c.Rps, | ||
| } | ||
|
|
||
| activityOptions, batchReq, err := opts.activityExecOrBatch(cctx, c.Parent.Namespace, cl, c.Yes, singleOrBatchOverrides{ | ||
| AllowYesWithActivityID: true, | ||
| }) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it make sense to do this validation before the
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is also similar to workflow delete command handler. I think one reason to keep it after describe namespace could be that if the describe namespace fails, the process still continues after a "global namespace warning". But if there is an error in execorbatch cmd parsing then it will exit. So if both errors are triggered on a cmd line, then moving the the validation before, will suppress the namespace warning. |
||
|
|
||
| if activityOptions != nil { | ||
| yes, err := cctx.promptYes(activityDeleteSingleConfirmationMessage(activityOptions), c.Yes) | ||
| if err != nil { | ||
| return err | ||
| } else if !yes { | ||
| return fmt.Errorf("user denied confirmation") | ||
| } | ||
| _, err = cl.WorkflowService().DeleteActivityExecution(cctx, &workflowservice.DeleteActivityExecutionRequest{ | ||
| Namespace: c.Parent.Namespace, | ||
| ActivityId: c.ActivityId, | ||
| RunId: c.RunId, | ||
| }) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to delete standalone activity: %w", err) | ||
| } | ||
| cctx.Printer.Println("Delete activity succeeded") | ||
| } else { // batchReq != nil | ||
| deleteActivitiesOperation := &batch.BatchOperationDeleteActivities{} | ||
| batchReq.Reason = cmp.Or(c.Reason, defaultReason()) | ||
| batchReq.Operation = &workflowservice.StartBatchOperationRequest_DeleteActivitiesOperation{ | ||
| DeleteActivitiesOperation: deleteActivitiesOperation, | ||
| } | ||
|
|
||
| if err := startBatchJob(cctx, cl, batchReq); err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func activityDeleteSingleConfirmationMessage(activityOptions *client.GetActivityHandleOptions) string { | ||
| action := fmt.Sprintf("Delete Standalone Activity %q", activityOptions.ActivityID) | ||
| if activityOptions.RunID != "" { | ||
| action += fmt.Sprintf(" with Run ID %q", activityOptions.RunID) | ||
| } | ||
| return fmt.Sprintf("%s? y/N", action) | ||
| } | ||
|
|
||
| func (c *TemporalActivityCompleteCommand) run(cctx *CommandContext, args []string) error { | ||
| cl, err := dialClient(cctx, &c.Parent.ClientOptions) | ||
| if err != nil { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't find it easy to understand the function name here:
activityExecOrBatchThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can change the name, if you have a better suggestion. I borrowed the name from existing
workflowExecOrBatchthat has similar function for batch workflow commands.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should keep it consistent of
workflowExecOrBatchoractivityExecOrBatch.