mjsax commented on code in PR #13909:
URL: https://github.com/apache/kafka/pull/13909#discussion_r1451069044
##########
clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java:
##########
@@ -1263,6 +1327,8 @@ protected String getConfigValue(ConfigKey key, String
headerName) {
return key.type.toString().toLowerCase(Locale.ROOT);
case "Default":
if (key.hasDefault()) {
+ if (key.alternativeString != null)
Review Comment:
Can we really add this here? `getConfigValue` might be used somewhere else,
and it might be expect to get the actual config value back, but never the
html-string.
I think we should ensure that the html-string is only returned on the call
path of `toHtml`?
##########
clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java:
##########
@@ -1238,6 +1277,31 @@ public ConfigKey(String name, Type type, Object
defaultValue, Validator validato
this.displayName = displayName;
this.recommender = recommender;
this.internalConfig = internalConfig;
+ this.overwrittenValue = null;
+ }
+
+ public ConfigKey(String name, Type type, Object defaultValue,
Validator validator,
Review Comment:
What I means was, to add parameter `String alternativeString` to the
existing constructor and pass in `null` on all existing callers which don't set
it. This way, we can still keep the member variable final but avoid this second
constructor which is a lot of boilerplate code.
--
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]
