ShivsundarR commented on code in PR #19417:
URL: https://github.com/apache/kafka/pull/19417#discussion_r2033807782
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ShareConsumerImpl.java:
##########
@@ -1085,18 +1062,18 @@ private void ensureExplicitAcknowledgement() {
*/
private static AcknowledgementMode
initializeAcknowledgementMode(ConsumerConfig config, Logger log) {
if (config == null) {
- return AcknowledgementMode.UNKNOWN;
+ return AcknowledgementMode.IMPLICIT;
}
- String acknowledgementModeStr =
config.getString(ConsumerConfig.INTERNAL_SHARE_ACKNOWLEDGEMENT_MODE_CONFIG);
- if ((acknowledgementModeStr == null) ||
acknowledgementModeStr.isEmpty()) {
- return AcknowledgementMode.UNKNOWN;
- } else if (acknowledgementModeStr.equalsIgnoreCase("implicit")) {
+ String acknowledgementModeStr =
config.getString(ConsumerConfig.SHARE_ACKNOWLEDGEMENT_MODE_CONFIG);
+ if ((acknowledgementModeStr == null) ||
+ acknowledgementModeStr.isEmpty() ||
+ acknowledgementModeStr.equalsIgnoreCase("implicit")) {
return AcknowledgementMode.IMPLICIT;
} else if (acknowledgementModeStr.equalsIgnoreCase("explicit")) {
return AcknowledgementMode.EXPLICIT;
}
- log.warn("Invalid value for config {}: \"{}\"",
ConsumerConfig.INTERNAL_SHARE_ACKNOWLEDGEMENT_MODE_CONFIG,
acknowledgementModeStr);
- return AcknowledgementMode.UNKNOWN;
+ log.warn("Invalid value for config, {}: \"{}\", setting mode to
default value : IMPLICIT", ConsumerConfig.SHARE_ACKNOWLEDGEMENT_MODE_CONFIG,
acknowledgementModeStr);
+ return AcknowledgementMode.IMPLICIT;
Review Comment:
For the case when the config had an invalid value, the code previously
logged a `WARN` log and returned `UNKNOWN`, now that we do not have `UNKNOWN`
mode anymore, I can think of 2 options here:
1. To throw an `IllegalArgumentException`, as the user should only enter
"implicit" or "explicit"
2. Log a warning and just assume the default value of `IMPLICIT`.
I have kept the 2nd option now, but I do feel in case of invalid config
entry, we need to throw an exception so that the user will correct the entry.
For empty/null values we can still assume `IMPLICIT`, but maybe we can throw
exception here?
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
