Jackie-Jiang commented on code in PR #14844:
URL: https://github.com/apache/pinot/pull/14844#discussion_r1966398184


##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotClusterConfigs.java:
##########
@@ -168,4 +171,73 @@ public SuccessResponse deleteClusterConfig(
       throw new ControllerApplicationException(LOGGER, errStr, 
Response.Status.INTERNAL_SERVER_ERROR, e);
     }
   }
+
+  @GET
+  @Path("/cluster/configs/groovy/staticAnalyzerConfig")
+  @Authorize(targetType = TargetType.CLUSTER, action = 
Actions.Cluster.GET_GROOVY_STATIC_ANALYZER_CONFIG)
+  @Produces(MediaType.APPLICATION_JSON)
+  @ApiOperation(value = "Get the configuration for Groovy Static analysis",
+      notes = "Get the configuration for Groovy static analysis")
+  @ApiResponses(value = {
+      @ApiResponse(code = 200, message = "Success"),
+      @ApiResponse(code = 500, message = "Internal server error")
+  })
+  public GroovyStaticAnalyzerConfig getGroovyStaticAnalysisConfig() throws 
Exception {

Review Comment:
   (format) Seems this doesn't follow [Pinot 
Style](https://docs.pinot.apache.org/developers/developers-and-contributors/code-setup#set-up-ide).
 Can you apply it and reformat the changes



##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java:
##########
@@ -29,6 +29,8 @@
 
 
 public class CommonConstants {
+  public static final String GROOVY_STATIC_ANALYZER_CONFIG = 
"pinot.server.groovy.static.analyzer";

Review Comment:
   Sounds good. Since we are not keeping the instance type prefix, we can put 
them in the top level, or add a subclass `Groovy`. Let's not put it in the 
beginning of the file, and also add some documentation for them.
   
   Regarding the REST API, the ser/de of this config is not easy, so I'd 
suggest keeping the REST API, but add a type param which can be `ingestion`, 
`query` or `all`. Same for the GET request, where we perform the same fallback 
logic based on the type asked



-- 
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

Reply via email to