hudi-agent commented on code in PR #18721:
URL: https://github.com/apache/hudi/pull/18721#discussion_r3243492733
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -3862,6 +3863,23 @@ private void validate() {
checkArgument(lookbackCommits >= 0,
String.format("%s must be non-negative, but was %d",
ROLLING_METADATA_TIMELINE_LOOKBACK_COMMITS.key(),
lookbackCommits));
+
+ validateEventTimeConfigs();
+ }
+
+ private void validateEventTimeConfigs() {
Review Comment:
🤖 nit: `validateEventTimeConfigs` reads like it enforces a constraint and
throws on failure — which is what other `validate*` methods in this builder
typically do. Since this one only logs a warning, a name like
`warnIfEventTimeWatermarkNotTracked` would more accurately signal the intent to
future readers.
<sub><i>- AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -3862,6 +3863,23 @@ private void validate() {
checkArgument(lookbackCommits >= 0,
String.format("%s must be non-negative, but was %d",
ROLLING_METADATA_TIMELINE_LOOKBACK_COMMITS.key(),
lookbackCommits));
+
+ validateEventTimeConfigs();
+ }
+
+ private void validateEventTimeConfigs() {
+ // Event-time field is configured but watermark tracking is off — the
field
+ // will be ignored at commit time. Surface a hint so the user can opt in
via
+ // TRACK_EVENT_TIME_WATERMARK.
+ String eventTimeFieldName =
writeConfig.getString(HoodiePayloadProps.PAYLOAD_EVENT_TIME_FIELD_PROP_KEY);
+ if (!StringUtils.isNullOrEmpty(eventTimeFieldName)
+ && !writeConfig.getBooleanOrDefault(TRACK_EVENT_TIME_WATERMARK)) {
+ log.warn("{}={} is configured but {}={}; event-time watermark metadata
will not be tracked. "
+ + "Set {}=true to record event-time watermark in commit
metadata.",
+ HoodiePayloadProps.PAYLOAD_EVENT_TIME_FIELD_PROP_KEY,
eventTimeFieldName,
+ TRACK_EVENT_TIME_WATERMARK.key(),
writeConfig.getBooleanOrDefault(TRACK_EVENT_TIME_WATERMARK),
Review Comment:
🤖 nit: `getBooleanOrDefault(TRACK_EVENT_TIME_WATERMARK)` is called again
here, but inside this branch it's always `false` (that's what the `if`
condition guards on). Extracting it to a local `boolean trackWatermark = ...`
before the `if` and reusing it in both places would avoid the redundant config
lookup and make the relationship explicit.
<sub><i>- AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
