codelipenghui commented on code in PR #16014:
URL: https://github.com/apache/pulsar/pull/16014#discussion_r895184770
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java:
##########
@@ -1418,12 +1418,13 @@ public synchronized PulsarClient getClient() throws
PulsarServerException {
ClientConfigurationData conf =
ConfigurationDataUtils.loadData(overrides,
initialConf, ClientConfigurationData.class);
- conf.setServiceUrl(this.getConfiguration().isTlsEnabled()
- ? this.brokerServiceUrlTls :
this.brokerServiceUrl);
-
conf.setTlsAllowInsecureConnection(this.getConfiguration().isTlsAllowInsecureConnection());
-
conf.setTlsTrustCertsFilePath(this.getConfiguration().getTlsCertificateFilePath());
+ boolean tlsEnabled =
this.getConfiguration().isBrokerClientTlsEnabled();
Review Comment:
Looks like the reason why we can't use the `isTlsEnabled` here is that we
can have tls port and non-tls port for a broker? It's better to add a
description here to clarify it.
And, please also add the compatibility explanation for the change. Or do we
need to consider the compatibility issue? Since users can upgrade to a new
version broker but with the old version config file.
The document of `brokerClientTlsEnabled` also needs to update since we also
use broker clients for system topics.
--
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]
