You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/03/03 03:23:20 UTC
[GitHub] [incubator-pinot] Jackie-Jiang opened a new pull request #6632: Do not log warning when gRPC or admin port is not configured for an instance
Jackie-Jiang opened a new pull request #6632:
URL: https://github.com/apache/incubator-pinot/pull/6632
Only log warning when gRPC or admin port is mis-configured. It is normal if they are not configured (e.g. they do not apply to controller, broker and minion)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6632: Do not log warning when gRPC or admin port is not configured for an instance
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #6632:
URL: https://github.com/apache/incubator-pinot/pull/6632#discussion_r587931997
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotInstanceRestletResource.java
##########
@@ -114,25 +109,27 @@ public String getInstance(
}
private int getGrpcPort(InstanceConfig instanceConfig) {
- int grpcPort;
- try {
- grpcPort = Integer.parseInt(instanceConfig.getRecord().getSimpleField(CommonConstants.Helix.Instance.GRPC_PORT_KEY));
- } catch (Exception e) {
- LOGGER.warn("Grpc port is not set for instance: {}", instanceConfig.getInstanceName(), e);
- grpcPort = Instance.NOT_SET_GRPC_PORT_VALUE;
+ String grpcPortStr = instanceConfig.getRecord().getSimpleField(CommonConstants.Helix.Instance.GRPC_PORT_KEY);
Review comment:
`instanceConfig` won't be `null` here, so just checking `grpcPortStr` should be enough
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on a change in pull request #6632: Do not log warning when gRPC or admin port is not configured for an instance
Posted by GitBox <gi...@apache.org>.
fx19880617 commented on a change in pull request #6632:
URL: https://github.com/apache/incubator-pinot/pull/6632#discussion_r587910406
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotInstanceRestletResource.java
##########
@@ -114,25 +109,27 @@ public String getInstance(
}
private int getGrpcPort(InstanceConfig instanceConfig) {
- int grpcPort;
- try {
- grpcPort = Integer.parseInt(instanceConfig.getRecord().getSimpleField(CommonConstants.Helix.Instance.GRPC_PORT_KEY));
- } catch (Exception e) {
- LOGGER.warn("Grpc port is not set for instance: {}", instanceConfig.getInstanceName(), e);
- grpcPort = Instance.NOT_SET_GRPC_PORT_VALUE;
+ String grpcPortStr = instanceConfig.getRecord().getSimpleField(CommonConstants.Helix.Instance.GRPC_PORT_KEY);
Review comment:
do we need to do any null check here?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang merged pull request #6632: Do not log warning when gRPC or admin port is not configured for an instance
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged pull request #6632:
URL: https://github.com/apache/incubator-pinot/pull/6632
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org