KAFKA-17164: Enforce 'application.server' <server>:<port> format at config level.#22202
KAFKA-17164: Enforce 'application.server' <server>:<port> format at config level.#22202chickenchickenlove wants to merge 2 commits intoapache:trunkfrom
Conversation
|
Hi, @Nikita-Shupletsov ! It’s been a while since we last spoke. |
nileshkumar3
left a comment
There was a problem hiding this comment.
@chickenchickenlove thanks for the PR. I have a very minor comment.
|
|
||
| final String endPoint = (String) value; | ||
|
|
||
| if (endPoint.isEmpty()) { |
There was a problem hiding this comment.
This is redundant , Utils.isBlank(endPoint) covers this.
|
@nileshkumar3 Thanks for the time to take a look! |
Nikita-Shupletsov
left a comment
There was a problem hiding this comment.
Thank you for the PR!
left a few comments
|
|
||
| private static final ConfigDef.Validator INSTANCE = new ApplicationServerConfigValidator(); | ||
|
|
||
| public static ConfigDef.Validator getInstance() { |
There was a problem hiding this comment.
is it worth having it as a singleton?
I checked a few places(e.g. https://github.com/apache/kafka/blob/trunk/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/TopicCreationConfig.java#L128) and it looks like we don't really do that anywhere else
| import org.apache.kafka.common.config.ConfigException; | ||
| import org.apache.kafka.common.utils.Utils; | ||
|
|
||
| import static org.apache.kafka.common.utils.Utils.getHost; |
There was a problem hiding this comment.
nit: some methods from this class are imported statically, some are called via Utils.method
| final Integer port; | ||
| try { | ||
| port = getPort(endPoint); | ||
| } catch (final NumberFormatException e) { |
There was a problem hiding this comment.
do we need to catch NumberFormatException? looks like HOST_PORT_PATTERN already checks that it's exclusively digits
There was a problem hiding this comment.
The pattern only says the port looks like digits. Integer.parseInt still errors if that number is too big for a int.
Description
This PR implement KIP-1245, which enforces the
application.serverhost:port format duringStreamsConfigconstruction.Changes
ConfigDef.Validatorforapplication.serverto fail fast on invalid values inStreamsConfiginstead of later inHostInfo.StreamsConfigTestcase to use a validapplication.servervalue.application.servervalues.