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

[UX] Print better error message when transfer validation fails #535

Merged
merged 5 commits into from
Sep 13, 2022

Conversation

xutingl
Copy link
Contributor

@xutingl xutingl commented Sep 12, 2022

Fixes #508

TransferFailedException raised by ReplicatorClient.verify_transfer_prefix() will now be caught in both cp and sync, and a detailed transfer failed message will be printed.
transfer_failed

catch TransferFailedException
@xutingl xutingl requested a review from parasj September 12, 2022 01:07
@parasj
Copy link
Contributor

parasj commented Sep 12, 2022

@liuxt129 Before you merge, can you take a look and see why it says "Transfer successfully completed" and then immediately prints "Transfer failed exception"?

Also, the error message should be more descriptive and say something like "The following objects were not found at the destination"

@xutingl
Copy link
Contributor Author

xutingl commented Sep 13, 2022

The "Transfer successfully completed" message is printed in function launch_replication_job in cp_replicate.py, before the verify is done.

If we don't want this to happen, I think we can move the success message part from cp_replicate.py to cli.py, after the verify passes. We have everything we need for the success message from stats returned from launch_replication_job

@xutingl
Copy link
Contributor Author

xutingl commented Sep 13, 2022

A more descriptive error message:
transfer_failed

report transfer_failed; descriptive error msg
@@ -41,7 +41,7 @@ def __init__(self, message, failed_objects=None):
def pretty_print_str(self):
err = f"[red][bold]:x: TransferFailedException:[/bold] {str(self)}[/red]"
if self.failed_objects and len(self.failed_objects) > 0:
err += "\n[bold][red]Failed objects:[/red][/bold] " + str(self.failed_objects)
err += "\n[bold][red]The following objects were not found at the destination:[/red][/bold] " + str(self.failed_objects)
Copy link
Contributor

Choose a reason for hiding this comment

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

"Transfer failed. The following..."

@parasj parasj changed the title catch TransferFailedException [UX] Print better error message when transfer validation fails Sep 13, 2022
print success msg after verify

print success msg after verify

print success msg after verify

print success msg after verify
@xutingl
Copy link
Contributor Author

xutingl commented Sep 13, 2022

New failed message:
transfer_failed

New success message:
transfer_succeeded

@xutingl xutingl enabled auto-merge (squash) September 13, 2022 23:34
@xutingl xutingl merged commit 704d3ce into main Sep 13, 2022
@xutingl xutingl deleted the dev/post_transfer_verif branch September 13, 2022 23:36
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.

Post-transfer Verification: Print out clearer message if verification fails
2 participants