PTEUDO-1543: Handle schema drops on restore failure, avoid special extensions #319
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
leandrorichardtoledo
commented
Sep 26, 2024
leandrorichardtoledo
commented
- Ticket: PTEUDO-1543;
# Conflicts: # pkg/databaseclaim/databaseclaim.go
EvertonCalgarotto
approved these changes
Sep 26, 2024
drewwells
reviewed
Sep 26, 2024
| @@ -1165,7 +1165,7 @@ func (r *DatabaseClaimReconciler) manageUserAndExtensions(reqInfo *requestInfo, | |||
| if err != nil { | |||
| return err | |||
| } | |||
| if roleCreated { | |||
| if roleCreated && operationalMode != M_MigrateExistingToNewDB && operationalMode != M_MigrationInProgress { | |||
drewwells
reviewed
Sep 26, 2024
drewwells
reviewed
Sep 26, 2024
pkg/pgctl/pgrestore.go
Outdated
| result.Error = &ResultError{Err: exitError, ExitCode: exitError.ExitCode(), CmdOutput: result.Output} | ||
|
|
||
| // Attempt to drop schemas after restore failure. | ||
| fmt.Println("restore failed, dropping all schemas") |
There was a problem hiding this comment.
If this is needed, can you pass in a logger to output the message?
There was a problem hiding this comment.
Removed
drewwells
suggested changes
Sep 26, 2024
pkg/pgctl/pgrestore.go
Outdated
| @@ -76,3 +107,25 @@ func (x *Restore) SetOptions(o []string) { | |||
| func (x *Restore) GetOptions() []string { | |||
| return x.Options | |||
| } | |||
|
|
|||
| // DropSchemas drops all schemas except the system ones. | |||
| func (x *Restore) DropSchemas() error { | |||
There was a problem hiding this comment.
The pattern of the other methods is to return a Result. Maybe that's a better option so stdout/stderr of the command can be handled more strictly.
Suggested change
| func (x *Restore) DropSchemas() error { | |
| func (x *Restore) DropSchemas() Result { |
There was a problem hiding this comment.
Changed
pkg/pgctl/pgrestore.go
Outdated
Comment on lines
123
to
129
| cmd := exec.Command(PSQL, x.DsnUri, "-c", dropSchemaSQL) | ||
| cmd.Stderr = os.Stderr | ||
| cmd.Stdout = os.Stdout | ||
|
|
||
| if err := cmd.Run(); err != nil { | ||
| return fmt.Errorf("error dropping schemas: %w", err) | ||
| } |
There was a problem hiding this comment.
Let's handle these with loggers rather than sending directly to stdout/stderr. You could copy what's done in (x *Dump) Exec or send back (string, error)
Suggested change
| cmd := exec.Command(PSQL, x.DsnUri, "-c", dropSchemaSQL) | |
| cmd.Stderr = os.Stderr | |
| cmd.Stdout = os.Stdout | |
| if err := cmd.Run(); err != nil { | |
| return fmt.Errorf("error dropping schemas: %w", err) | |
| } | |
| var result Result | |
| cmd := exec.Command(PSQL, x.DsnUri, "-c", dropSchemaSQL) | |
| stderrIn, _ := cmd.StderrPipe() | |
| go func() { | |
| result.Output = streamExecOutput(stderrIn, opts) | |
| }() | |
| cmd.Start() | |
| err := cmd.Wait() | |
| ... |
pkg/pgctl/pgrestore.go
Outdated
| if dropErr != nil { | ||
| result.Error = &ResultError{ | ||
| Err: dropErr, | ||
| CmdOutput: fmt.Sprintf("restore error: %v\ndrop schemas error: %v", exitError, dropErr), |
There was a problem hiding this comment.
The error is already in Err field, no reason to duplicate it. You've also lost the stdout/stderr text. It should be passing hat back or just have DropSchemas() return a Result{} object
drewwells
approved these changes
Sep 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.