Skip to content
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

Single-workflow Signal Command #418

Closed
wants to merge 6 commits into from
Closed

Conversation

cretz
Copy link
Member

@cretz cretz commented Jan 25, 2024

What was changed

This demonstrates how to write a simple command. Admittedly it didn't have to do much output or output testing and it could have more testing. Also, this does not include the "batch" capabilities of existing CLI.

@cretz cretz requested a review from a team January 25, 2024 19:28
if err != nil {
return fmt.Errorf("failed signalling workflow: %w", err)
}
cctx.Printer.Println("Signal workflow succeeded")
Copy link
Member Author

Choose a reason for hiding this comment

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

Up for discussion on whether this is a log statement or a normal line. If --output json is specified, there will be no output regardless.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not personally expecting a non-blocking request-response command such as signal to have a concept of "logging" -- I'm just expecting stdout and stderr.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 This matches existing CLI where, in the absence of more detail, it just prints a line of success.

Copy link
Contributor

@dandavison dandavison left a comment

Choose a reason for hiding this comment

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

Let's merge into the main branch so we can review there.

Base automatically changed from cli-rewrite-first to cli-rewrite January 30, 2024 14:35
…write-workflow-signal

# Conflicts:
#	go.mod
#	go.sum
#	temporalcli/commands.gen.go
#	temporalcli/commands.workflow.go
#	temporalcli/commands.workflow_exec.go
#	temporalcli/commandsmd/commands.md
@cretz
Copy link
Member Author

cretz commented Jan 30, 2024

Closing this in favor of a new PR pointing to the cli-rewrite branch (coming soon)

@cretz cretz closed this Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants