turcsanyip commented on code in PR #9993:
URL: https://github.com/apache/nifi/pull/9993#discussion_r2142792088
##########
nifi-extension-bundles/nifi-kafka-bundle/nifi-kafka-shared/src/main/java/org/apache/nifi/kafka/shared/component/KafkaClientComponent.java:
##########
@@ -32,12 +32,11 @@ public interface KafkaClientComponent {
PropertyDescriptor BOOTSTRAP_SERVERS = new PropertyDescriptor.Builder()
.name("bootstrap.servers")
- .displayName("Kafka Brokers")
- .description("Comma-separated list of Kafka Brokers in the format
host:port")
+ .displayName("Bootstrap Servers")
+ .description("Comma-separated list of Kafka Bootstrap Servers in
the format host:port. Corresponds to Kafka bootstrap.servers property")
.required(true)
- .addValidator(StandardValidators.HOSTNAME_PORT_LIST_VALIDATOR)
- .expressionLanguageSupported(ExpressionLanguageScope.ENVIRONMENT)
- .defaultValue("localhost:9092")
Review Comment:
Thanks for flagging this!
In general, I did not want to change the behavior of the properties in order
to eliminate compatibility issues at all. So I took the property definitions
from `Kafka3ConnectionService` because those were used effectively.
On the other hand, it makes sense to add `HOSTNAME_PORT_LIST_VALIDATOR`
because the existing `Bootstrap Servers` property values must be following this
format already. Otherwise the property is unusable. So this seems a minimal
risk change and therefore I'm leaning toward adding the validator.
Regarding `localhost:9092` default value: The "localhost" values are
typically used during development only and they must be changed in real life
use cases almost all the time. Personally, I don't prefer "developer defaults"
exposed in prod apps (even if it is handy for devs). However, I see that we use
"localhost" defaults in some places so if you think it is kind of a convention
in NiFi, I can add it.
So I'm +1 for the validator and rather -1 for the default value. Please
share what you think based on the background info provided above and I will
change the code accordingly.
--
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]
