suryaprasanna commented on code in PR #17859:
URL: https://github.com/apache/hudi/pull/17859#discussion_r2722758096
##########
hudi-common/src/main/java/org/apache/hudi/common/fs/FSUtils.java:
##########
@@ -314,6 +314,17 @@ public static String createNewFileId(String idPfx, int id)
{
.toString();
}
+ public static StoragePath getPartitionPath(StoragePath basePath, String
partition) {
Review Comment:
Done.
##########
hudi-common/src/main/java/org/apache/hudi/common/fs/FSUtils.java:
##########
@@ -314,6 +314,17 @@ public static String createNewFileId(String idPfx, int id)
{
.toString();
}
+ public static StoragePath getPartitionPath(StoragePath basePath, String
partition) {
+ return StringUtils.isNullOrEmpty(partition)
+ ? basePath : new StoragePath(basePath, partition);
+ }
+
+ public static StoragePath getFilePath(StoragePath basePath, String
partition, String fileName) {
Review Comment:
Done.
##########
hudi-hadoop-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestTable.java:
##########
@@ -723,6 +723,16 @@ public HoodieTestTable withPartitionMetaFiles(List<String>
partitionPaths) throw
return this;
}
+ public HoodieTestTable withPartitionMetaFilesWithDepth(String...
partitionPaths) throws IOException {
+ for (String partitionPath : partitionPaths) {
+ StoragePath partitionPathObj = FSUtils.getPartitionPath(new
StoragePath(basePath), partitionPath);
+ HoodiePartitionMetadata partitionMetadata1 = new
HoodiePartitionMetadata(storage,
+ currentInstantTime, new StoragePath(basePath), partitionPathObj,
Option.of(HoodieFileFormat.PARQUET));
Review Comment:
Yeah, it is optional. Removing it now.
##########
hudi-hadoop-mr/src/main/java/org/apache/hudi/hadoop/HoodieROTablePathFilter.java:
##########
@@ -192,10 +216,10 @@ public boolean accept(Path path) {
// which contains old version files, if not specify this value,
these files will be filtered.
fsView =
FileSystemViewManager.createInMemoryFileSystemViewWithTimeline(engineContext,
metaClient, HoodieInputFormatUtils.buildMetadataConfig(conf),
-
metaClient.getActiveTimeline().filterCompletedInstants().findInstantsBeforeOrEquals(timestampAsOf));
+ completedTimeline.findInstantsBeforeOrEquals(timestampAsOf));
} else {
- fsView =
FileSystemViewManager.createInMemoryFileSystemView(engineContext,
Review Comment:
It was always completedTimeline, not getCommitsTimeline. Even before my
change we are using
`metaClient.getActiveTimeline().filterCompletedInstants()` not
`metaClient.getActiveTimeline().getCommitsTimeline().filterCompletedInstants()`
##########
hudi-hadoop-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestTable.java:
##########
@@ -723,6 +723,16 @@ public HoodieTestTable withPartitionMetaFiles(List<String>
partitionPaths) throw
return this;
}
+ public HoodieTestTable withPartitionMetaFilesWithDepth(String...
partitionPaths) throws IOException {
Review Comment:
There is already withPartitionMetaFiles method in the same class. So,
withPartitionMetaFilesWithDepth makes more sense, changing this method to
addPartitionMetaFiles can create confusion, and we may need to refactor other
method names as well.Let me know what yo think?
##########
hudi-hadoop-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestTable.java:
##########
@@ -723,6 +723,16 @@ public HoodieTestTable withPartitionMetaFiles(List<String>
partitionPaths) throw
return this;
}
+ public HoodieTestTable withPartitionMetaFilesWithDepth(String...
partitionPaths) throws IOException {
+ for (String partitionPath : partitionPaths) {
+ StoragePath partitionPathObj = FSUtils.getPartitionPath(new
StoragePath(basePath), partitionPath);
+ HoodiePartitionMetadata partitionMetadata1 = new
HoodiePartitionMetadata(storage,
Review Comment:
My bad, refactoring gone wrong. Fixed it now.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
