chia7712 commented on code in PR #20334:
URL: https://github.com/apache/kafka/pull/20334#discussion_r2507680776
##########
docs/upgrade.html:
##########
@@ -113,6 +113,35 @@ <h5><a id="upgrade_420_notable"
href="#upgrade_420_notable">Notable changes in 4
<li>
The <code>num.replica.fetchers</code> config has a new lower bound of
1.
</li>
+ <li>
+ Improvements have been made to the validation rules and default values
of LIST-type configurations
+ (<a href="https://cwiki.apache.org/confluence/x/HArXF";>KIP-1161</a>).
+ <ul>
+ <li>
+ LIST-type configurations now enforce stricter validation:
+ <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>
Review Comment:
> Thinking a bit more about this. Since we typically only introduce breaking
changes in major releases, it might be useful to take a more conservative
approach by only logging a warning for duplicated entries for now and rejecting
them starting in 5.0.
perhaps we should print warnings instead of throwing exception in 4.x.
```java
if (Set.copyOf(values).size() != values.size()) {
throw new ConfigException("Configuration '" + name + "'
values must not be duplicated.");
}
```
and then we could change it to exception in 5.x
##########
docs/upgrade.html:
##########
@@ -113,6 +113,35 @@ <h5><a id="upgrade_420_notable"
href="#upgrade_420_notable">Notable changes in 4
<li>
The <code>num.replica.fetchers</code> config has a new lower bound of
1.
</li>
+ <li>
+ Improvements have been made to the validation rules and default values
of LIST-type configurations
+ (<a href="https://cwiki.apache.org/confluence/x/HArXF";>KIP-1161</a>).
+ <ul>
+ <li>
+ LIST-type configurations now enforce stricter validation:
+ <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>
Review Comment:
> Thinking a bit more about this. Since we typically only introduce breaking
changes in major releases, it might be useful to take a more conservative
approach by only logging a warning for duplicated entries for now and rejecting
them starting in 5.0.
perhaps we should print warnings instead of throwing exception in 4.x.
```java
if (Set.copyOf(values).size() != values.size()) {
throw new ConfigException("Configuration '" + name + "'
values must not be duplicated.");
}
```
and then we could change it to exception in 5.x
@junrao @m1a2st WDYT?
--
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]
