Skip to content

Conversation

veceraj
Copy link
Contributor

@veceraj veceraj commented Jul 29, 2025

PBM-1574 Powered by Pull Request Badge

The --wait flag is supported, but it doesn't wait long enough.

This PR moves the dumpMeta call into the defer block. Previously, dumpMeta() was called before r.close() completed. Since r.close() can take time, the restore could finish early from the CLI's perspective.

@radoslawszulgo radoslawszulgo requested a review from Copilot August 22, 2025 13:25
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes an issue where the --wait flag in PBM restore was not waiting for the complete restoration process to finish. The main change moves the dumpMeta call into the defer block to ensure it runs after r.close() completes, since r.close() can take significant time.

  • Moved dumpMeta call from the main function body into the defer block
  • Added proper error handling for the close operation
  • Ensures the restore metadata is written only after all cleanup operations complete

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

err = errors.Wrap(err, "snapshot close")
cerr := r.close(noerr, progress)

if cerr != nil && err == nil {
Copy link
Preview

Copilot AI Aug 22, 2025

Choose a reason for hiding this comment

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

[nitpick] The error handling logic could be simplified. Consider using a more straightforward approach where you preserve the original error if it exists, otherwise use the close error.

Suggested change
if cerr != nil && err == nil {
if err == nil && cerr != nil {

Copilot uses AI. Check for mistakes.

err = errors.Wrap(cerr, "snapshot close")
}

if noerr && err == nil {
Copy link
Preview

Copilot AI Aug 22, 2025

Choose a reason for hiding this comment

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

The condition noerr && err == nil is redundant since noerr is defined as err == nil. You can simplify this to just if noerr {.

Suggested change
if noerr && err == nil {
if err == nil {

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant