-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
HIVE-28608: Resolve Issues in ReplicationMigrationTool and Activate Test Suite #5595
Conversation
Quality Gate passedIssues Measures |
@ayushtkn Can you help to review. Thanks |
@deniskuzZ @ayushtkn @okumin Please help to review the PR. Thanks |
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.
+1. I'm waiting for 24 hours to follow the guideline
@@ -155,6 +155,9 @@ public int run(String[] args) throws Exception { | |||
|
|||
System.out.println("Completed verification. Source & Target are " + (failed == 0 ? "in Sync." : "not in Sync.")); | |||
System.out.println("Time Taken: " + (System.currentTimeMillis() - startTime) + " ms"); | |||
if (failed != 0) { | |||
return -1; | |||
} |
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.
👍
@@ -359,7 +362,6 @@ public boolean validateAtFileLevel(Path srcDir, Path tgtDir, FileSystem srcFs, F | |||
LocatedFileStatus sourceFile = srcListing.next(); | |||
if (filtersPattern != null && !isCopied(sourceFile.getPath(), filtersPattern)) { | |||
LOG.info("Entry: {} is filtered.", sourceFile.getPath()); | |||
continue; |
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.
I understand this is just a cleanup that doesn't change behaviors
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.
you are right
@okumin Can you help to check this PR |
What changes were proposed in this pull request?
Fixed minor bugs in ReplicationMigrationTool and enabled TestReplicationMigrationTool class
Why are the changes needed?
The testcases were not running properly from beginning and went unnoticed because of lower Maven surefire plugin which suppressed the errors. Post HIVE-28519 the test cases started failing. Error log is added in the jira.
Does this PR introduce any user-facing change?
No
Is the change a dependency upgrade?
No
How was this patch tested?
Existing test case