Workflow: List -> Show, and Expand Metadata#6544
Conversation
sougou
left a comment
sougou
left a comment
There was a problem hiding this comment.
This is forward progress. We still need to get the routing rules. But that can be a separate PR. I'm slightly hesitant about the MasterReplicationStatuses, but understand why, and can't think of something better. I'll let @rohit-nayak-ps also take a look to make sure there are no better alternatives.
go/vt/wrangler/vexec.go
Outdated
There was a problem hiding this comment.
I'd recommend using an existing flag. *topo.RemoteOperationTimeout looks appropriate.
There was a problem hiding this comment.
Good call, I'll swap it out.
|
I've included an expanded |
There was a problem hiding this comment.
Looks good. The only remaining concern is whether we are allowed to change the command name options like this or we should be marking List as deprecated and allowing it as a synonym for Show.
@rohit-nayak-ps we did not explicitly mark VExec and Workflow as experimental in 7.0. What are your thoughts on rename vs deprecation?
…mmand, and changed the command from List to Show. Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
PrismaPhonic
force-pushed
the
workflow-show-update
branch
from
2b789f0 to
1e51804
Compare
Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
… it's a subcommand. Signed-off-by: Peter Farr <Peter@PrismaPhonic.com>
|
I built in accepting |
enisoc
left a comment
enisoc
left a comment
There was a problem hiding this comment.
I don't know this code well enough to approve, but the comments I made earlier are now resolved so I have no objections.
| return err | ||
| } | ||
| if action == "list" || action == "listall" { | ||
| if action == "show" || action == "listall" { |
There was a problem hiding this comment.
Do we not print anything out for show / listall?
There was a problem hiding this comment.
@rohit-nayak-ps this doesn't look right. However, it is unrelated to the changes in this PR so I'm going to move this forward.
There was a problem hiding this comment.
The printing happens internally actually. From what I can tell this is a way of handling split code paths from subcommands.
There was a problem hiding this comment.
As Peter replied, show/listall follows a separate path and outputs a json doc. Otherwise we just return rows affected (result of vexec for stop/start/delete workflow)
6 participants
This PR renames the
Listsubcommand toShowand expands the metadata to also returnTabletControlsand whether the master of each shard is currently serving write traffic. This PR also changed thelistStreamsmethod to be namedShowWorkflowto fit the new subcommand nomenclature, and makes it public sovitess-operatorcan use this wrangler method, and expose replication status information in a k8s friendly way.