scylladb / scylla-manager

The Scylla Manager
https://manager.docs.scylladb.com/stable/
Other
48 stars 33 forks source link

docs: explain --all-clusters in backup list #3811

Closed karol-kokoszka closed 2 months ago

karol-kokoszka commented 2 months ago

Fixes #3780


Please make sure that:

karol-kokoszka commented 2 months ago

@annastuchlik Could you please take a look at this PR ? There were a lot of question asking if customer can somehow remove the backup of non-existing cluster from the backup location. They actually can do it with the manager by listing the backups first and then removing the one that is not needed. sctool backup list requires to pass --cluster-id though, what may not intuitive at the first glance. There is an explanation for that and I included this explanation into the CLI description.

annastuchlik commented 2 months ago

@karol-kokoszka I need to understand the problem better before I provide any feedback. My understanding is that there are two issues with the backup list command:

I'm guessing because the description explains how it works under the hood, but it's unclear to me what the user should do.

Next:

There were a lot of question asking if customer can somehow remove the backup of non-existing cluster from the backup location.

This PR updates the description of the backup list command. How is that related to removing backups? Should the description of backup delete be updated, too?

Finally, perhaps we should add instructions on how to remove backups to the Backup page.

karol-kokoszka commented 2 months ago

If you're using the --all-clusters option, you still need to provide the --cluster-id of one of the clusters (and that cluster will work as a coordinator for listing backups of other clusters). You need to provide --cluster-id of an existing cluster managed by Scylla Manager. If the cluster is deleted, the command will fail.

That's correct.

I'm guessing because the description explains how it works under the hood, but it's unclear to me what the user should do.

That's very good point, and I agree with you that the explanation should be added to the general backup description. Will do the followup commit just right after.

This PR updates the description of the backup list command. How is that related to removing backups? Should the description of backup delete be updated, too?

Makes sense, let me update backup delete too.

Finally, perhaps we should add instructions on how to remove backups to the Backup page.

Yes, will add the commit.

@annastuchlik Thanks for the follow up questions !

karol-kokoszka commented 2 months ago

@annastuchlik PR updated. Please have a look.

karol-kokoszka commented 2 months ago

@annastuchlik ping. I appreciate your view on the documentation and don't want to merge changes without it.

annastuchlik commented 2 months ago

@karol-kokoszka I've left two suggestions for language improvements, and I'm approving this PR, as all the information is there. Thanks!