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: Improve help message for ws restore command #1659

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion backend/capellacollab/cli/ws.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,16 @@ def backup(
@app.command()
def restore(
volume_name: str,
tarfile: t.Annotated[pathlib.Path, typer.Argument(exists=True)],
tarfile: t.Annotated[
pathlib.Path,
typer.Argument(
exists=True,
help=(
"gzip compressed file with one top level "
"directory named 'workspace'"
Comment on lines +196 to +197
Copy link
Member

@MoritzWeber0 MoritzWeber0 Jul 18, 2024

Choose a reason for hiding this comment

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

Suggested change
"gzip compressed file with one top level "
"directory named 'workspace'"
"tarfile generated by the backup CLI command"

I wouldn't support manually created tarfiles officially, there is a lot more that can go wrong. The purpose of the command is to restore backups done by the CLI itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd rather keep it as proposed. I can imagine scenarios where people

1
backup a workpace

2
modify the backup

3
compress the modified backup

4
restore the packed workspace

The CLI can deal with a manually archived workspace folder. This is good and useful. It just lacks documentation here.

Copy link
Member

Choose a reason for hiding this comment

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

Accepting a custom archive would mean that we also need proper validation in the restore interface to provide a correct error message in case of wrongly packed archives. Instead, I'd propose to add two new CLI functions: pack and unpack. Unpack takes the archive and unpacks it into a target directory. Pack takes the directory and packs it again.

),
),
],
namespace: t.Annotated[str, NamespaceOption],
access_mode: str = "ReadWriteMany",
storage_class_name: str = "persistent-sessions-csi",
Expand Down
Loading