Clarify backup and restore creation messages #270
Conversation
|
cc @Bradamant3 |
|
Jennifer plans to review this tomorrow |
pkg/cmd/cli/backup/create.go
Outdated
| @@ -143,6 +143,7 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { | |||
| return err | |||
| } | |||
|
|
|||
| fmt.Printf("Backup %q created successfully.\n", backup.Name) | |||
| fmt.Printf("Backup request %q submitted successfully.\n", backup.Name) | |||
| fmt.Printf("Use `ark backup describe %s` for more details.\n", backup.Name) | |||
There was a problem hiding this comment.
Nit: s/Use/Run
(same for restore)
reason: "use" is vague, "run" is more precise.
There was a problem hiding this comment.
Also, maybe this is my ignorance showing, but does ark backup describe ... provide additional information about the status of the backup request? If so, maybe let's ponder further ...
There was a problem hiding this comment.
What we have now is ark backup get and ark backup describe. get shows you the basic details in tabular format (the essential info being the phase - new/in progress/failed/completed). describe shows you slightly more information, but it's mostly the spec - what namespaces/resources/etc are included in the backup. Until we have #20 implemented, there's not really any info you can get other than seeing phase transitions.
|
@Bradamant3 Here's what |
|
@Bradamant3 Updated the messages |
pkg/cmd/cli/restore/create.go
Outdated
| @@ -144,6 +144,7 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { | |||
| return err | |||
| } | |||
|
|
|||
| fmt.Printf("Restore %q created successfully.\n", restore.Name) | |||
| fmt.Printf("Restore request for %q submitted successfully.\n", restore.Name) | |||
There was a problem hiding this comment.
I feel like this should either say "Restore request <restore.Name>", or "Restore request for <o.BackupName>" - WDYT?
There was a problem hiding this comment.
I think if/until we change to ark restore create <name> --backup <backup name>, the output here must include at least the restore's generated name. Since that includes the backup's name, I think the former (just removing "for") is probably better.
There was a problem hiding this comment.
Ok, can do that. I used “for” as I was viewing it was the request for the restore object to be created. However, the request isn’t really an object in Ark so I suppose it makes sense to drop the “for”.
When running `ark <resource> create`, a request is sent to the server, but the status is not immediately known. Inform the user that a request was sent and provide a way to get more information on it. Signed-off-by: Nolan Brubaker <nolan@heptio.com>
|
@Bradamant3 please take a final look - thanks! |
When running
ark <resource> create, a request is sent to the server,but the status is not immediately known. Inform the user that a request
was sent and provide a way to get more information on it.
I'm open to changing the message text if others think of something better.
Fixes #232