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

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

Merged
merged 2 commits into from
Apr 30, 2024

Conversation

karol-kokoszka
Copy link
Collaborator

Fixes #3780


Please make sure that:

  • Code is split to commits that address a single change
  • Commit messages are informative
  • Commit titles have module prefix
  • Commit titles have issue nr. suffix

@karol-kokoszka
Copy link
Collaborator Author

@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 annastuchlik added the documentation Improvements or additions to documentation label Apr 18, 2024
@annastuchlik
Copy link
Collaborator

@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:

  • 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.

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

@annastuchlik PR updated.
Please have a look.

@karol-kokoszka
Copy link
Collaborator Author

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

@annastuchlik
Copy link
Collaborator

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

@karol-kokoszka karol-kokoszka merged commit 9bff200 into master Apr 30, 2024
51 of 57 checks passed
@karol-kokoszka karol-kokoszka deleted the docs_update_backup_list branch April 30, 2024 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'sctool backup list --all-clusters` requires cluster id
2 participants