m1a2st commented on code in PR #20844:
URL: https://github.com/apache/kafka/pull/20844#discussion_r2521480892
##########
clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java:
##########
@@ -316,18 +316,22 @@ public Map<String, Object>
valuesWithPrefixOverride(String prefix) {
String keyWithNoPrefix =
entry.getKey().substring(prefix.length());
ConfigDef.ConfigKey configKey =
definition.configKeys().get(keyWithNoPrefix);
if (configKey != null)
- result.put(keyWithNoPrefix,
definition.parseValue(configKey, entry.getValue(), true));
+ result.put(keyWithNoPrefix,
definition.parseValue(configKey, entry.getValue(), true,
allowDuplicateValueInList()));
else {
String keyWithNoSecondaryPrefix =
keyWithNoPrefix.substring(keyWithNoPrefix.indexOf('.') + 1);
configKey =
definition.configKeys().get(keyWithNoSecondaryPrefix);
if (configKey != null)
- result.put(keyWithNoPrefix,
definition.parseValue(configKey, entry.getValue(), true));
+ result.put(keyWithNoPrefix,
definition.parseValue(configKey, entry.getValue(), true,
allowDuplicateValueInList()));
}
}
}
return result;
}
+ protected boolean allowDuplicateValueInList() {
Review Comment:
Using the `allowDuplicateValueInList` tag isn’t a good approach. I think we
can change it to check whether `key.validator` is an instance of `ValidList` in
`parseValue`. For the` LogConfig.validate` method, it should directly call the
`ConfigDef.parse` method instead of using `getList()`.
##########
docs/upgrade.html:
##########
@@ -139,7 +139,9 @@ <h5><a id="upgrade_420_notable"
href="#upgrade_420_notable">Notable changes in 4
<ul>
<li>Null values are no longer accepted for most LIST-type
configurations, except those that explicitly
allow a null default value or where a null value has a
well-defined semantic meaning.</li>
- <li>Duplicate entries within the same list are no longer
permitted.</li>
+ <li>Most LIST-type configurations no longer accept
duplicate entries, except in cases where duplicates
Review Comment:
> Should we change ValidList.ensureValid() to only log a warning for
duplicated values?
I think we can keep the exception-throwing behavior, but we now preprocess
the user’s configuration to remove duplicate entries.
> The following makes the generated "Valid Values" in html quite verbose.
I have updated the document
<img width="645" height="355" alt="asdfsfd"
src="https://github.com/user-attachments/assets/8402a0e3-53aa-46ae-9cf2-2c3cd2be8ab0";
/>
<img width="631" height="198" alt="sdaf"
src="https://github.com/user-attachments/assets/128a633a-50a5-4406-89ae-5b67defa35db";
/>
<img width="636" height="279" alt="dlfsakfds"
src="https://github.com/user-attachments/assets/8f605f42-3706-45f4-af3b-71b6a7ec90f6";
/>
--
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]
