Copilot commented on code in PR #17575:
URL: https://github.com/apache/pinot/pull/17575#discussion_r2728398783
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java:
##########
@@ -1070,6 +1070,7 @@ public enum Type {
public static class Grpc {
public static final String KEY_OF_GRPC_PORT = "pinot.broker.grpc.port";
+ public static final int DEFAULT_BROKER_GRPC_PORT = 8010;
Review Comment:
The constant DEFAULT_BROKER_GRPC_PORT is defined here but never used in this
PR. The quickstart code hardcodes the value 8010 directly instead of
referencing this constant. Consider using this constant in
QuickstartRunner.java at line 57 to maintain consistency and avoid magic
numbers.
##########
pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/StartBrokerCommand.java:
##########
@@ -48,6 +48,10 @@ public class StartBrokerCommand extends
AbstractBaseAdminCommand implements Comm
@CommandLine.Option(names = {"-brokerPort"}, required = false, description =
"Broker port number to use for query.")
private int _brokerPort = CommonConstants.Helix.DEFAULT_BROKER_QUERY_PORT;
+ @CommandLine.Option(names = {"-brokerGrpcPort"}, required = false,
description = "Broker gRPC port number to use "
+ + "for query. Disabled by default.")
Review Comment:
The description should clarify the behavior of different port values:
positive values enable gRPC on that port, 0 enables gRPC on an available port,
and negative values (default -1) disable gRPC entirely. This would help users
understand how to configure the option.
```suggestion
+ "for query. Positive values enable gRPC on that port, 0 enables gRPC
on an available port, and negative "
+ "values (default -1) disable gRPC.")
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
