xushiyan commented on code in PR #7037:
URL: https://github.com/apache/hudi/pull/7037#discussion_r1014078055


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/HoodieTable.java:
##########
@@ -858,10 +860,22 @@ public final Option<HoodieTableMetadataWriter> 
getMetadataWriter(String triggeri
 
   /**
    * Check if action type is a table service.
-   * @param actionType action type of interest.
+   * @param instant instant of interest
    * @return true if action represents a table service. false otherwise.
    */
-  public abstract boolean isTableServiceAction(String actionType);
+  public boolean isTableServiceAction(HoodieInstant instant) {
+    if (instant.getAction().equals(HoodieTimeline.REPLACE_COMMIT_ACTION)) {
+      Option<Pair<HoodieInstant, HoodieClusteringPlan>> instantPlan = 
ClusteringUtils.getClusteringPlan(getMetaClient(), instant);

Review Comment:
   ```suggestion
         Option<Pair<HoodieInstant, HoodieClusteringPlan>> instantPlan = 
ClusteringUtils.getClusteringPlan(metaClient, instant);
   ```



##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/BaseActionExecutor.java:
##########
@@ -58,7 +59,7 @@ public BaseActionExecutor(HoodieEngineContext context, 
HoodieWriteConfig config,
    */
   protected final void writeTableMetadata(HoodieCommitMetadata metadata, 
String actionType) {
     table.getMetadataWriter(instantTime).ifPresent(w -> w.update(
-        metadata, instantTime, table.isTableServiceAction(actionType)));
+        metadata, instantTime, table.isTableServiceAction(new 
HoodieInstant(HoodieInstant.State.INFLIGHT, actionType, instantTime))));

Review Comment:
   not a big fan of creating dummy instant to cater the API. it's the API that 
needs to fit better with the caller info.



##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java:
##########
@@ -656,7 +659,9 @@ private List<DirectoryInfo> 
listAllPartitions(HoodieTableMetaClient datasetMetaC
           partitionsToBootstrap.add(dirInfo);
         } else {
           // Add sub-dirs to the queue
-          pathsToList.addAll(dirInfo.getSubDirectories());
+          pathsToList.addAll(dirInfo.getSubDirectories().stream()
+              .map(path -> new SerializablePath(new CachingPath(path.toUri())))
+              .collect(Collectors.toList()));
         }

Review Comment:
   unrelated changes?



-- 
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]

Reply via email to