Jackie-Jiang commented on code in PR #9471:
URL: https://github.com/apache/pinot/pull/9471#discussion_r992591294
##########
pinot-clients/pinot-jdbc-client/src/main/java/org/apache/pinot/client/controller/PinotControllerTransportFactory.java:
##########
@@ -80,6 +81,7 @@ public PinotControllerTransportFactory
withConnectionProperties(Properties prope
DEFAULT_CONTROLLER_CONNECT_TIMEOUT_MS));
_handshakeTimeoutMs =
Integer.parseInt(properties.getProperty("controllerHandshakeTimeoutMs",
DEFAULT_CONTROLLER_HANDSHAKE_TIMEOUT_MS));
+ _appId = properties.getProperty("appId", "");
Review Comment:
Same here, we want to set `null` when it is not configured
```suggestion
_appId = properties.getProperty("appId");
```
##########
pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/BrokerCache.java:
##########
@@ -104,6 +104,7 @@ public BrokerCache(Properties properties, String
controllerUrl) {
DEFAULT_CONTROLLER_CONNECT_TIMEOUT_MS));
int handshakeTimeoutMs =
Integer.parseInt(properties.getProperty("controllerHandshakeTimeoutMs",
DEFAULT_CONTROLLER_HANDSHAKE_TIMEOUT_MS));
+ String appId = properties.getProperty("appId",
DEFAULT_CONTROLLER_HANDSHAKE_TIMEOUT_MS);
Review Comment:
```suggestion
String appId = properties.getProperty("appId");
```
##########
pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/JsonAsyncHttpPinotClientTransportFactory.java:
##########
@@ -94,6 +96,7 @@ public JsonAsyncHttpPinotClientTransportFactory
withConnectionProperties(Propert
DEFAULT_BROKER_CONNECT_TIMEOUT_MS));
_handshakeTimeoutMs =
Integer.parseInt(properties.getProperty("brokerHandshakeTimeoutMs",
DEFAULT_BROKER_HANDSHAKE_TIMEOUT_MS));
+ _appId = properties.getProperty("appId", "");
Review Comment:
^^
##########
pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/JsonAsyncHttpPinotClientTransport.java:
##########
@@ -66,7 +66,8 @@ public JsonAsyncHttpPinotClientTransport() {
}
public JsonAsyncHttpPinotClientTransport(Map<String, String> headers, String
scheme,
- @Nullable SSLContext sslContext, ConnectionTimeouts connectionTimeouts,
TlsProtocols tlsProtocols) {
+ @Nullable SSLContext sslContext,
ConnectionTimeouts connectionTimeouts,
Review Comment:
The format is still not correct. Please update the IDE style setting and
reformat
##########
pinot-clients/pinot-jdbc-client/src/main/java/org/apache/pinot/client/controller/PinotControllerTransport.java:
##########
@@ -64,21 +66,26 @@ public PinotControllerTransport(Map<String, String>
headers, String scheme,
builder.setReadTimeout(connectionTimeouts.getReadTimeoutMs())
.setConnectTimeout(connectionTimeouts.getConnectTimeoutMs())
.setHandshakeTimeout(connectionTimeouts.getHandshakeTimeoutMs())
- .setUserAgent(getUserAgentVersionFromClassPath())
+ .setUserAgent(getUserAgentVersionFromClassPath(appId))
.setEnabledProtocols(tlsProtocols.getEnabledProtocols().toArray(new String[0]));
_httpClient = Dsl.asyncHttpClient(builder.build());
}
- private String getUserAgentVersionFromClassPath() {
+ private String getUserAgentVersionFromClassPath(@Nullable String appId) {
Properties userAgentProperties = new Properties();
try {
userAgentProperties.load(PinotControllerTransport.class.getClassLoader()
.getResourceAsStream("version.properties"));
} catch (IOException e) {
LOGGER.warn("Unable to set user agent version");
}
- return userAgentProperties.getProperty("ua", "unknown");
+ String userAgentFromProperties = userAgentProperties.getProperty("ua",
"unknown");
+ if (StringUtils.isNotEmpty(appId)) {
+ return appId.substring(0,
Math.min(org.apache.pinot.client.utils.ConnectionUtils.APP_ID_MAX_CHARS,
Review Comment:
(minor) import `ConnectionUtils`
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
