codope commented on code in PR #19464:
URL: https://github.com/apache/hudi/pull/19464#discussion_r3718134192
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieTableServiceClient.java:
##########
@@ -644,7 +645,7 @@ private void completeClustering(HoodieReplaceCommitMetadata
replaceCommitMetadat
heartbeatClient.stop(clusteringCommitTime);
}
log.info("Clustering successfully on commit {} for table {}",
clusteringCommitTime, table.getConfig().getBasePath());
- fireCommitCallbackIfNecessary(clusteringCommitTime,
clusteringInstant.getAction(),
+ fireCommitCallbackIfNecessary(clusteringCommitTime,
completedClusteringInstant.getAction(),
Review Comment:
> for normal commits, the commit action type is inferred from
`CommitUtils.getCommitActionType`
True for ingestion writes and the overwrite operation types, but it does not
extend to table services. If you see
`TestCommitUtils#testNormalOperationsDelegateBasedOnTableType` excludes only
the four overwrite ops from its EnumSource, so it asserts this for CLUSTER and
COMPACT today.
> we can just pass around the REPLACE_COMMIT_ACTION or just drop this param
It is a field on `HoodieWriteCommitCallbackMessage` (@PublicAPIClass, added
by HUDI-7161), and the built-in `HoodieWriteCommitHttpCallback` serialises the
message to JSON and POSTs it, a remote consumer has no timeline to filter. I
would not prefer changing the public api. However, on just passing around
REPLACE_COMMIT_ACTION, that does work. The only difference is copying the value
versus reading the one the timeline just wrote. I preferred the latter so the
mapping keeps a single definition, but I don't feel strongly. If you prefer I
can switch to the constant and revert the util to return void. let me know.
--
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]
