You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "walterddr (via GitHub)" <gi...@apache.org> on 2023/04/11 15:54:12 UTC

[GitHub] [pinot] walterddr commented on a diff in pull request #10588: Make broker's rest resource packages configurable

walterddr commented on code in PR #10588:
URL: https://github.com/apache/pinot/pull/10588#discussion_r1163025688


##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java:
##########
@@ -225,6 +225,10 @@ public static class Broker {
     public static final String CONFIG_OF_BROKER_ID = "pinot.broker.instance.id";
     public static final String CONFIG_OF_BROKER_HOSTNAME = "pinot.broker.hostname";
     public static final String CONFIG_OF_SWAGGER_USE_HTTPS = "pinot.broker.swagger.use.https";
+    // Comma separated list of packages that contains javax service resources.
+    public static final String BROKER_RESOURCE_PACKAGES = "broker.restlet.api.resource.packages";
+    public static final String DEFAULT_BROKER_RESOURCE_PACKAGES = "org.apache.pinot.broker.api.resources";

Review Comment:
   it's a bit weird to me that the controller resource package config is actually in `ControllerConf` class and i cant find an equivalent `BrokerConf` class.



##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java:
##########
@@ -259,6 +263,7 @@ public static class Broker {
 
     public static final String DISABLE_GROOVY = "pinot.broker.disable.query.groovy";
     public static final boolean DEFAULT_DISABLE_GROOVY = true;
+    public static final String DEFAULT_BROKER_RESOURCE_PACKAGE = "org.apache.pinot.broker.api.resources";

Review Comment:
   this is redundant right?



##########
pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/StartBrokerCommand.java:
##########
@@ -67,6 +67,7 @@ public class StartBrokerCommand extends AbstractBaseAdminCommand implements Comm
       // TODO: support forbids = {"-brokerHost", "-brokerPort"})
   private String _configFileName;
 
+  @CommandLine.Option(names = {"-configOverrides"}, required = false, description = "Proxy config overrides")

Review Comment:
   what's the intent for this commandline options?



-- 
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-unsubscribe@pinot.apache.org

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