-
Notifications
You must be signed in to change notification settings - Fork 107
PBM-1600 - Interactive confirm prompt when restore #1180
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
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -49,6 +49,7 @@ type restoreOpts struct { | |||||
| nsFrom string | ||||||
| nsTo string | ||||||
| usersAndRoles bool | ||||||
| confirmYes bool | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The confirmYes field is added to the struct but there's no visible command-line flag definition or initialization code in this diff. Ensure that a corresponding CLI flag (like --yes or --confirm) is properly defined and wired to this field. Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
| rsMap string | ||||||
| conf string | ||||||
| ts string | ||||||
|
|
@@ -91,10 +92,9 @@ No other pbm command is available while the restore is running! | |||||
| `, | ||||||
| r.Snapshot, r.Name) | ||||||
| } | ||||||
| return fmt.Sprintf("Restore of the snapshot from '%s' has started", r.Snapshot) | ||||||
| return fmt.Sprintf("\nRestore of the snapshot from '%s' has started", r.Snapshot) | ||||||
| case r.PITR != "": | ||||||
| return fmt.Sprintf("Restore to the point in time '%s' has started", r.PITR) | ||||||
|
|
||||||
| return fmt.Sprintf("\nRestore to the point in time '%s' has started", r.PITR) | ||||||
| default: | ||||||
| return "" | ||||||
| } | ||||||
|
|
@@ -449,15 +449,10 @@ func doRestore( | |||||
| } | ||||||
| err = yaml.UnmarshalStrict(buf, &cmd.Restore.ExtConf) | ||||||
| if err != nil { | ||||||
| return nil, errors.Wrap(err, "unable to unmarshal config file") | ||||||
| return nil, errors.Wrap(err, "unable to unmarshal config file") | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| err = sendCmd(ctx, conn, cmd) | ||||||
| if err != nil { | ||||||
| return nil, errors.Wrap(err, "send command") | ||||||
| } | ||||||
|
|
||||||
| if outf != outText { | ||||||
| return &restore.RestoreMeta{ | ||||||
| Name: name, | ||||||
|
|
@@ -477,7 +472,23 @@ func doRestore( | |||||
| if o.pitr != "" { | ||||||
| pitrs = fmt.Sprintf(" to point-in-time %s", o.pitr) | ||||||
| } | ||||||
| fmt.Printf("Starting restore %s%s%s", name, pitrs, bcpName) | ||||||
|
|
||||||
| fmt.Println("Restore:") | ||||||
| fmt.Printf(" - %s%s%s\n", name, pitrs, bcpName) | ||||||
|
|
||||||
| if !o.confirmYes { | ||||||
| err := util.AskConfirmation("Are you sure you want to restore this backup?") | ||||||
| if err != nil { | ||||||
| return nil, err | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| err = sendCmd(ctx, conn, cmd) | ||||||
| if err != nil { | ||||||
| return nil, errors.Wrap(err, "send command") | ||||||
| } | ||||||
|
|
||||||
| fmt.Printf("Starting restore") | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The message "Starting restore" lacks context compared to the original message that included the backup name and other details. Consider including key restore information in this status message for better user experience.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
|
|
||||||
| var ( | ||||||
| fn getRestoreMetaFn | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,39 @@ | ||||||||
| package util | ||||||||
|
|
||||||||
| import ( | ||||||||
| "bufio" | ||||||||
| "fmt" | ||||||||
| "os" | ||||||||
| "runtime" | ||||||||
| "strings" | ||||||||
|
|
||||||||
| "github.com/percona/percona-backup-mongodb/pbm/errors" | ||||||||
| ) | ||||||||
|
|
||||||||
| func AskConfirmation(question string) error { | ||||||||
| fi, err := os.Stdin.Stat() | ||||||||
| if err != nil { | ||||||||
| return errors.Wrap(err, "stat stdin") | ||||||||
| } | ||||||||
| if (fi.Mode() & os.ModeCharDevice) == 0 { | ||||||||
| return errors.New("no tty") | ||||||||
| } | ||||||||
|
|
||||||||
| if runtime.GOOS == "linux" { | ||||||||
| question = fmt.Sprintf("\033[1;33m%s\033[0m", question) // Yellow text for Linux terminals | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The color formatting is only applied to Linux systems, but many other Unix-like systems (macOS, FreeBSD, etc.) also support ANSI color codes. Consider using a more inclusive condition like
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||
| } | ||||||||
| fmt.Printf("%s [y/N] ", question) | ||||||||
|
|
||||||||
| scanner := bufio.NewScanner(os.Stdin) | ||||||||
| scanner.Scan() | ||||||||
| if err := scanner.Err(); err != nil { | ||||||||
| return errors.Wrap(err, "read stdin") | ||||||||
| } | ||||||||
|
|
||||||||
| switch strings.TrimSpace(scanner.Text()) { | ||||||||
| case "yes", "Yes", "YES", "Y", "y": | ||||||||
| return nil | ||||||||
| } | ||||||||
|
|
||||||||
| return errors.ErrUserCanceled | ||||||||
| } | ||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import should follow Go conventions by being grouped with other third-party imports and maintaining alphabetical order. Move this import to be grouped with other project imports.
Copilot uses AI. Check for mistakes.