pkumarsinha commented on a change in pull request #1936:
URL: https://github.com/apache/hive/pull/1936#discussion_r592395172
##########
File path:
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosExternalTablesMetaDataOnly.java
##########
@@ -639,9 +629,11 @@ public void testIncrementalDumpEmptyDumpDirectory() throws
Throwable {
.verifyResult(inc2Tuple.lastReplicationId);
}
- private void assertFalseExternalFileList(Path externalTableFileList)
- throws IOException {
+ private void assertFalseExternalFileList(String dumpLocation)
Review comment:
Can you please move this method to ReplicationTestUtils itself. We have
duplicate code for this method on two classes.
##########
File path:
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosExternalTablesMetaDataOnly.java
##########
@@ -639,9 +629,11 @@ public void testIncrementalDumpEmptyDumpDirectory() throws
Throwable {
.verifyResult(inc2Tuple.lastReplicationId);
}
- private void assertFalseExternalFileList(Path externalTableFileList)
- throws IOException {
+ private void assertFalseExternalFileList(String dumpLocation)
+ throws IOException {
Review comment:
I think it doesn't throw IOException
##########
File path:
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosAcrossInstances.java
##########
@@ -2225,17 +2224,11 @@ private void setupUDFJarOnHDFS(Path
identityUdfLocalPath, Path identityUdfHdfsPa
/*
* Method used from TestReplicationScenariosExclusiveReplica
*/
- private void assertExternalFileInfo(List<String> expected, String
dumplocation, boolean isIncremental,
+ private void assertExternalFileList(List<String> expected, String
dumplocation,
WarehouseInstance warehouseInstance)
throws IOException {
Path hivePath = new Path(dumplocation, ReplUtils.REPL_HIVE_BASE_DIR);
- Path metadataPath = new Path(hivePath, EximUtil.METADATA_PATH_NAME);
- Path externalTableInfoFile;
- if (isIncremental) {
- externalTableInfoFile = new Path(hivePath, FILE_NAME);
- } else {
- externalTableInfoFile = new Path(metadataPath,
primaryDbName.toLowerCase() + File.separator + FILE_NAME);
- }
- ReplicationTestUtils.assertExternalFileInfo(warehouseInstance, expected,
externalTableInfoFile);
+ Path externalTblFileList = new Path(hivePath, EximUtil.FILE_LIST_EXTERNAL);
Review comment:
This is still not addressed. The same method(and code) is defined in
three classes. assertExternalFileList. And essentially they aren't doing more
than the path formation. As discussed, can we not use the
ReplicationTestUtils.assertExternalFileList directly by modifying the signature
a bit.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
