[ https://issues.apache.org/jira/browse/HIVE-24718?focusedWorklogId=564628&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-564628 ]
ASF GitHub Bot logged work on HIVE-24718: ----------------------------------------- Author: ASF GitHub Bot Created on: 11/Mar/21 14:25 Start Date: 11/Mar/21 14:25 Worklog Time Spent: 10m Work Description: 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: us...@infra.apache.org Issue Time Tracking ------------------- Worklog Id: (was: 564628) Time Spent: 6h (was: 5h 50m) > Moving to file based iteration for copying data > ----------------------------------------------- > > Key: HIVE-24718 > URL: https://issues.apache.org/jira/browse/HIVE-24718 > Project: Hive > Issue Type: Bug > Reporter: Arko Sharma > Assignee: Arko Sharma > Priority: Major > Labels: pull-request-available > Attachments: HIVE-24718.01.patch, HIVE-24718.02.patch, > HIVE-24718.04.patch, HIVE-24718.05.patch, HIVE-24718.06.patch > > Time Spent: 6h > Remaining Estimate: 0h > -- This message was sent by Atlassian Jira (v8.3.4#803005)