From edb5151b59085d48b1847834170a375e4d9001d9 Mon Sep 17 00:00:00 2001 From: Doug Davis Date: Wed, 8 Apr 2020 06:35:01 -0400 Subject: [PATCH] Remove the delete propagation flag (#770) * Remove the delete propagation flag In it's current state it now takes me about 25 seconds for the `kn delete` to complete. Before https://github.com/knative/client/pull/682 it used to be almost immediate. This is because we now pass in the `DeletePropagationBackground` flag. I believe this is a mistake, not only because of the 20+ seconds of additional time to delete things, but IMO the CLI should talk to the server in the same way regardless of the --wait flag. That flag should just be a CLI thing to indicate if the user wants the CLI to wait for the server to complete but not HOW the server should do the delete. Signed-off-by: Doug Davis * try just tweaking the --no-wait flag Signed-off-by: Doug Davis --- docs/cmd/kn_revision_delete.md | 2 +- docs/cmd/kn_service_delete.md | 2 +- pkg/kn/commands/wait_flags.go | 9 ++++++++- pkg/kn/commands/wait_flags_test.go | 9 +++++++++ 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/docs/cmd/kn_revision_delete.md b/docs/cmd/kn_revision_delete.md index 5d6f4e2c41..1337acb4c4 100644 --- a/docs/cmd/kn_revision_delete.md +++ b/docs/cmd/kn_revision_delete.md @@ -24,7 +24,7 @@ kn revision delete NAME [flags] --async DEPRECATED: please use --no-wait instead. Delete revision and don't wait for it to be deleted. -h, --help help for delete -n, --namespace string Specify the namespace to operate in. - --no-wait Delete revision and don't wait for it to be deleted. + --no-wait Delete revision and don't wait for it to be deleted. (default true) --wait-timeout int Seconds to wait before giving up on waiting for revision to be deleted. (default 600) ``` diff --git a/docs/cmd/kn_service_delete.md b/docs/cmd/kn_service_delete.md index 971fd9314b..6a2aa9f720 100644 --- a/docs/cmd/kn_service_delete.md +++ b/docs/cmd/kn_service_delete.md @@ -27,7 +27,7 @@ kn service delete NAME [flags] --async DEPRECATED: please use --no-wait instead. Delete service and don't wait for it to be deleted. -h, --help help for delete -n, --namespace string Specify the namespace to operate in. - --no-wait Delete service and don't wait for it to be deleted. + --no-wait Delete service and don't wait for it to be deleted. (default true) --wait-timeout int Seconds to wait before giving up on waiting for service to be deleted. (default 600) ``` diff --git a/pkg/kn/commands/wait_flags.go b/pkg/kn/commands/wait_flags.go index ca78368568..d92ee6f13f 100644 --- a/pkg/kn/commands/wait_flags.go +++ b/pkg/kn/commands/wait_flags.go @@ -42,8 +42,15 @@ type WaitFlags struct { func (p *WaitFlags) AddConditionWaitFlags(command *cobra.Command, waitTimeoutDefault int, action, what, until string) { waitUsage := fmt.Sprintf("%s %s and don't wait for it to be %s.", action, what, until) //TODO: deprecated flag should be removed in next release + + // Special-case 'delete' command so it comes back to the user immediately + noWaitDefault := false + if action == "Delete" { + noWaitDefault = true + } + command.Flags().BoolVar(&p.Async, "async", false, "DEPRECATED: please use --no-wait instead. "+waitUsage) - command.Flags().BoolVar(&p.NoWait, "no-wait", false, waitUsage) + command.Flags().BoolVar(&p.NoWait, "no-wait", noWaitDefault, waitUsage) timeoutUsage := fmt.Sprintf("Seconds to wait before giving up on waiting for %s to be %s.", what, until) command.Flags().IntVar(&p.TimeoutInSeconds, "wait-timeout", waitTimeoutDefault, timeoutUsage) diff --git a/pkg/kn/commands/wait_flags_test.go b/pkg/kn/commands/wait_flags_test.go index 9c711d2c78..ce6ad0004f 100644 --- a/pkg/kn/commands/wait_flags_test.go +++ b/pkg/kn/commands/wait_flags_test.go @@ -115,3 +115,12 @@ func TestAddWaitUsageMessage(t *testing.T) { t.Error("wrong until message") } } + +func TestAddWaitUsageDelete(t *testing.T) { + flags := &WaitFlags{} + cmd := cobra.Command{} + flags.AddConditionWaitFlags(&cmd, 60, "Delete", "blub", "deleted") + if !strings.Contains(cmd.UsageString(), "deleted. (default true)") { + t.Error("Delete has wrong default value for --no-wait") + } +}