hachikuji commented on a change in pull request #11691:
URL: https://github.com/apache/kafka/pull/11691#discussion_r794736312
##########
File path:
clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java
##########
@@ -226,6 +227,10 @@ public Password getPassword(String key) {
return keys;
}
+ public boolean originalsContainsKey(String configKey) {
Review comment:
AbstractConfig is sadly a public class. Can we do the PR without this?
##########
File path:
clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java
##########
@@ -514,11 +509,12 @@ public ProducerConfig(Map<String, Object> props) {
}
boolean idempotenceEnabled() {
- boolean userConfiguredIdempotence =
this.originals().containsKey(ENABLE_IDEMPOTENCE_CONFIG);
- boolean userConfiguredTransactions =
this.originals().containsKey(TRANSACTIONAL_ID_CONFIG);
- boolean idempotenceEnabled = userConfiguredIdempotence &&
this.getBoolean(ENABLE_IDEMPOTENCE_CONFIG);
- if (!idempotenceEnabled && userConfiguredIdempotence &&
userConfiguredTransactions)
+ boolean userConfiguredIdempotence =
this.originalsContainsKey(ENABLE_IDEMPOTENCE_CONFIG);
Review comment:
I think this can be simplified a little bit more by getting rid of this
variable. How about this?
```java
boolean userConfiguredTransactions =
this.originalsContainsKey(TRANSACTIONAL_ID_CONFIG);
boolean idempotenceEnabled = this.getBoolean(ENABLE_IDEMPOTENCE_CONFIG);
if (!idempotenceEnabled && userConfiguredTransactions) {
throw new ConfigException("Cannot set a " +
ProducerConfig.TRANSACTIONAL_ID_CONFIG + " without also enabling idempotence.");
}
return idempotenceEnabled;
--
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
