Workflow: Add List-All Command #6533
Conversation
…space. Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
There was a problem hiding this comment.
The cli list-all command has some minor issues. Otherwise LGTM, nice to be able to discover existing workflows!
- we need to provide a dummy workflow name to pass the keyspace.workflow validation ...
- in addition to the workflow list, a "no result returned" is tagged on
Line 2965 in 154baee
need to add list-all to the if condition
I tested by running list-all after exiting test/local_example.sh before 203_switch_reads.sh
$ kvtctl Workflow customer.xx list-all
Workflows: commerce2customerno result returned
$ kvtctl Workflow customer list-all
E0805 11:48:22.661750 10024 main.go:67] remote error: rpc error: code = Unknown desc = invalid format for <keyspace.workflow>: customer
$ alias kvtctl
alias kvtctl='vtctlclient -server localhost:15999'
go/vt/vtctl/vtctl.go
Outdated
| @@ -478,7 +478,7 @@ var commands = []commandGroup{ | |||
| "Workflow", []command{ | |||
| {"Workflow", commandWorkflow, | |||
| "<ks.workflow> <action> --dry-run", | |||
| "Start/Stop/Delete Workflow on all target tablets in workflow. Example: Workflow merchant.morders Start", | |||
| "Start/Stop/Delete/List/List-All Workflow on all target tablets in workflow. Example: Workflow merchant.morders Start", | |||
There was a problem hiding this comment.
Some suggested renames:
List->Show
List-All -> ListAll
There was a problem hiding this comment.
@sougou List was already there, but just not documented here in the text. It feels a bit out of scope to rename that subcommand in this PR. Are you ok with renaming it in another PR. Also @rohit-nayak-ps thoughts on the naming?
There was a problem hiding this comment.
What do you think the analog for ListAll is in the command itself? just listall with no hyphen?
There was a problem hiding this comment.
@PrismaPhonic, I will work on the List rename and parameter ordering changes Sugu suggested as part of some upcoming changes in the list response (need to report blacklisted tables, routing rules related to the workflow, among other info ...)
There was a problem hiding this comment.
Renamed List-All to ListAll as our subcommand naming convention is PascalCase.
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
|
@rohit-nayak-ps good catch. Fixed! |
PrismaPhonic
dismissed
rohit-nayak-ps’s stale review
Addressed changes requested.
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
PrismaPhonic
force-pushed
the
workflow-list-all
branch
from
1febfc3 to
bcc4cd8
Compare
This PR adds a
list-allsubcommand to the workflow command. Choosinglist-allwill return a list of all active workflows for the given keyspace.The justification for this is that consumers of Vitess (for instance
vitess-operator) will need the ability to discover active workflows, and not be required to hold onto workflow state themselves. Being able to discover workflows means that an operator can easily uncover whether resharding is currently ongoing, and act accordingly.