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

Vtctld Workflow command: minor fixes #7008

Merged
merged 3 commits into from Nov 17, 2020

Conversation

rohit-nayak-ps
Copy link
Contributor

@rohit-nayak-ps rohit-nayak-ps commented Nov 9, 2020

  • Workflow ListAll now includes stopped workflows. It was implemented to only give the active workflows as per PSDB's requirement. However this is confusing for regular users who don't see a created workflow if streams are in Stopped state.

  • Also improved output for ListAll: in particular, when there are no workflows an explicit message is provided instead of nothing

  • If an invalid workflow name is provided Workflow Show now errors out (earlier an inconsistent output was shown)

Signed-off-by: Rohit Nayak rohit@planetscale.com

Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
@rohit-nayak-ps rohit-nayak-ps marked this pull request as ready for review November 10, 2020 08:30
@rohit-nayak-ps
Copy link
Contributor Author

@PrismaPhonic, there is a breaking change: you will need to call wr.ListActiveWorkflows() instead of wr.ListAllWorkflows() in PSDB Sharding Workflows when you use a Vitess distro after this is merged. Also please check that any other change in this PR will not impact you.

@PrismaPhonic
Copy link
Contributor

Wouldn't this result in also revealing potentially a large number of FROZEN workflows that never got cleaned up?

@rohit-nayak-ps
Copy link
Contributor Author

Wouldn't this result in also revealing potentially a large number of FROZEN workflows that never got cleaned up?

Yes, it will be useful to get users to cleanup their frozen workflows as a best practice. Users have also begun to use Workflow Delete to cleanup, so in general we should not have a lot of FROZEN workflows in any case.

Copy link
Member

@deepthi deepthi left a comment

Choose a reason for hiding this comment

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

LGTM

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.

None yet

4 participants