-
Notifications
You must be signed in to change notification settings - Fork 42
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
CLI Refresh: workflow signal
command
#432
Changes from 9 commits
4e34c90
c4b1ff6
fcc6137
a4fcf9b
01ed375
2d07efc
c27034f
16d1e00
2f91ff0
3a7b209
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,16 @@ | ||
package temporalcli | ||
|
||
import "fmt" | ||
import ( | ||
"fmt" | ||
"os/user" | ||
|
||
"github.com/google/uuid" | ||
"github.com/temporalio/cli/temporalcli/internal/printer" | ||
"go.temporal.io/api/batch/v1" | ||
"go.temporal.io/api/common/v1" | ||
"go.temporal.io/api/workflowservice/v1" | ||
"go.temporal.io/sdk/client" | ||
) | ||
|
||
func (*TemporalWorkflowCancelCommand) run(*CommandContext, []string) error { | ||
return fmt.Errorf("TODO") | ||
|
@@ -22,8 +32,52 @@ func (*TemporalWorkflowResetBatchCommand) run(*CommandContext, []string) error { | |
return fmt.Errorf("TODO") | ||
} | ||
|
||
func (*TemporalWorkflowSignalCommand) run(*CommandContext, []string) error { | ||
return fmt.Errorf("TODO") | ||
func (c *TemporalWorkflowSignalCommand) run(cctx *CommandContext, args []string) error { | ||
cl, err := c.Parent.ClientOptions.dialClient(cctx) | ||
if err != nil { | ||
return err | ||
} | ||
defer cl.Close() | ||
|
||
// Get input payloads | ||
input, err := c.buildRawInputPayloads() | ||
if err != nil { | ||
return err | ||
} | ||
|
||
exec, batchReq, err := c.workflowExecOrBatch(cctx, c.Parent.Namespace, cl) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
// Run single or batch | ||
if exec != nil { | ||
// We have to use the raw signal service call here because the Go SDK's | ||
// signal call doesn't accept multiple arguments. | ||
_, err = cl.WorkflowService().SignalWorkflowExecution(cctx, &workflowservice.SignalWorkflowExecutionRequest{ | ||
Namespace: c.Parent.Namespace, | ||
WorkflowExecution: &common.WorkflowExecution{WorkflowId: c.WorkflowId, RunId: c.RunId}, | ||
SignalName: c.Name, | ||
Input: input, | ||
Identity: clientIdentity(), | ||
}) | ||
if err != nil { | ||
return fmt.Errorf("failed signalling workflow: %w", err) | ||
} | ||
cctx.Printer.Println("Signal workflow succeeded") | ||
} else if batchReq != nil { | ||
batchReq.Operation = &workflowservice.StartBatchOperationRequest_SignalOperation{ | ||
SignalOperation: &batch.BatchOperationSignal{ | ||
Signal: c.Name, | ||
Input: input, | ||
Identity: clientIdentity(), | ||
}, | ||
} | ||
if err := startBatchJob(cctx, cl, batchReq); err != nil { | ||
return err | ||
} | ||
} | ||
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. I believe that the contract of 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. Yes, this is leftover from previous code where it could return all nils when you declined the prompt, but I decided to change that to be an error and did not change this. |
||
return nil | ||
} | ||
|
||
func (*TemporalWorkflowStackCommand) run(*CommandContext, []string) error { | ||
|
@@ -41,3 +95,77 @@ func (*TemporalWorkflowTraceCommand) run(*CommandContext, []string) error { | |
func (*TemporalWorkflowUpdateCommand) run(*CommandContext, []string) error { | ||
return fmt.Errorf("TODO") | ||
} | ||
|
||
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. Although it's a private function, it would be good to document this something like
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. Concur 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. OK great, I can make these minor changes. |
||
func (s *SingleWorkflowOrBatchOptions) workflowExecOrBatch( | ||
cctx *CommandContext, | ||
namespace string, | ||
cl client.Client, | ||
) (*common.WorkflowExecution, *workflowservice.StartBatchOperationRequest, error) { | ||
// If workflow is set, we return single execution | ||
if s.WorkflowId != "" { | ||
if s.Query != "" { | ||
return nil, nil, fmt.Errorf("cannot set query when workflow ID is set") | ||
} else if s.Reason != "" { | ||
return nil, nil, fmt.Errorf("cannot set reason when workflow ID is set") | ||
} else if s.Yes { | ||
return nil, nil, fmt.Errorf("cannot set 'yes' when workflow ID is set") | ||
} | ||
return &common.WorkflowExecution{WorkflowId: s.WorkflowId, RunId: s.RunId}, nil, nil | ||
} | ||
|
||
// Check query is set properly | ||
if s.Query == "" { | ||
return nil, nil, fmt.Errorf("must set either workflow ID or query") | ||
} else if s.WorkflowId != "" { | ||
return nil, nil, fmt.Errorf("cannot set workflow ID when query is set") | ||
} else if s.RunId != "" { | ||
return nil, nil, fmt.Errorf("cannot set run ID when query is set") | ||
} | ||
|
||
// Count the workflows that will be affected | ||
count, err := cl.CountWorkflow(cctx, &workflowservice.CountWorkflowExecutionsRequest{Query: s.Query}) | ||
if err != nil { | ||
return nil, nil, fmt.Errorf("failed counting workflows from query: %w", err) | ||
} | ||
yes, err := cctx.promptYes( | ||
fmt.Sprintf("Start batch against approximately %v workflow(s)? y/N", count.Count), s.Yes) | ||
if err != nil { | ||
return nil, nil, err | ||
} else if !yes { | ||
// We consider this a command failure | ||
return nil, nil, fmt.Errorf("user denied confirmation") | ||
} | ||
|
||
// Default the reason if not set | ||
reason := s.Reason | ||
if reason == "" { | ||
username := "<unknown-user>" | ||
if u, err := user.Current(); err != nil && u.Username != "" { | ||
username = u.Username | ||
} | ||
reason = "Requested from CLI by " + username | ||
} | ||
Comment on lines
+139
to
+147
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. Current CLI requires reason to be set by user. UI defaults to something like |
||
|
||
return nil, &workflowservice.StartBatchOperationRequest{ | ||
Namespace: namespace, | ||
JobId: uuid.NewString(), | ||
VisibilityQuery: s.Query, | ||
Reason: reason, | ||
}, nil | ||
} | ||
|
||
func startBatchJob(cctx *CommandContext, cl client.Client, req *workflowservice.StartBatchOperationRequest) error { | ||
_, err := cl.WorkflowService().StartBatchOperation(cctx, req) | ||
if err != nil { | ||
return fmt.Errorf("failed starting batch operation: %w", err) | ||
} | ||
cctx.Logger.Info("Started batch", "jobId", req.JobId) | ||
if cctx.JSONOutput { | ||
return cctx.Printer.PrintStructured( | ||
struct { | ||
BatchJobID string `json:"batchJobId"` | ||
}{BatchJobID: req.JobId}, | ||
printer.StructuredOptions{}) | ||
} | ||
return 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.
Did this just happen to be the first command implemented for which batch was an option?
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.
Yes