yihua commented on code in PR #2864:
URL: https://github.com/apache/hudi/pull/2864#discussion_r3737907049
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/sources/helpers/KafkaOffsetGen.java:
##########
@@ -182,7 +182,7 @@ public KafkaOffsetGen(TypedProperties props) {
});
DataSourceUtils.checkRequiredProperties(props,
Collections.singletonList(Config.KAFKA_TOPIC_NAME));
topicName = props.getString(Config.KAFKA_TOPIC_NAME);
- String kafkaAutoResetOffsetsStr =
props.getString(Config.KAFKA_AUTO_RESET_OFFSETS,
Config.DEFAULT_KAFKA_AUTO_RESET_OFFSETS.name().toLowerCase());
+ String kafkaAutoResetOffsetsStr =
props.getString(Config.KAFKA_AUTO_OFFSET_RESET,
Config.DEFAULT_KAFKA_AUTO_OFFSET_RESET.name().toLowerCase());
Review Comment:
🤖 Now that this reads the genuine Kafka key `auto.offset.reset`, its valid
native values include `none` (in addition to earliest/latest). If a user sets
`auto.offset.reset=none` — a legal Kafka value the underlying consumer honors —
this loop won't find a matching KafkaResetOffsetStrategies entry and will throw
HoodieDeltaStreamerException. Should `none` (and any other native values) be
tolerated/passed through rather than rejected?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/sources/helpers/KafkaOffsetGen.java:
##########
@@ -157,9 +157,9 @@ public static class Config {
private static final String KAFKA_TOPIC_NAME =
"hoodie.deltastreamer.source.kafka.topic";
private static final String MAX_EVENTS_FROM_KAFKA_SOURCE_PROP =
"hoodie.deltastreamer.kafka.source.maxEvents";
- // "auto.reset.offsets" is kafka native config param. Do not change the
config param name.
- public static final String KAFKA_AUTO_RESET_OFFSETS = "auto.reset.offsets";
- private static final KafkaResetOffsetStrategies
DEFAULT_KAFKA_AUTO_RESET_OFFSETS = KafkaResetOffsetStrategies.LATEST;
+ // "auto.offset.reset" is kafka native config param. Do not change the
config param name.
+ public static final String KAFKA_AUTO_OFFSET_RESET = "auto.offset.reset";
Review Comment:
🤖 Since the key is changing from `auto.reset.offsets` to
`auto.offset.reset`, users who already set the old key will silently fall back
to the default (LATEST) rather than their intended value. Is it worth keeping
the old key as a recognized alternative, or at least calling this out in
release notes so existing pipelines don't silently change offset behavior?
<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]
