mayankshriv commented on a change in pull request #5949: URL: https://github.com/apache/incubator-pinot/pull/5949#discussion_r487550170
########## File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/ZookeeperResource.java ########## @@ -75,6 +80,63 @@ public String getData( return null; } + @DELETE + @Path("/zk/delete") + @Produces(MediaType.TEXT_PLAIN) + @ApiOperation(value = "Get content of the znode") + @ApiResponses(value = { // Review comment: Are the trailing `//` needed for formatting, or some other purpose? ########## File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/ZookeeperResource.java ########## @@ -75,6 +80,63 @@ public String getData( return null; } + @DELETE + @Path("/zk/delete") + @Produces(MediaType.TEXT_PLAIN) + @ApiOperation(value = "Get content of the znode") + @ApiResponses(value = { // + @ApiResponse(code = 200, message = "Success"), // + @ApiResponse(code = 404, message = "ZK Path not found"), // + @ApiResponse(code = 204, message = "No Content"), // + @ApiResponse(code = 500, message = "Internal server error")}) + public SuccessResponse delete( + @ApiParam(value = "Zookeeper Path, must start with /", required = true, defaultValue = "/") @QueryParam("path") @DefaultValue("") String path) { + + path = validateAndNormalizeZKPath(path); + + boolean success = pinotHelixResourceManager.deleteZKPath(path); + if(success) { + return new SuccessResponse("Successfully deleted path: " + path); + } else { + throw new ControllerApplicationException(LOGGER, "Failed to delete path: " + path, + Response.Status.INTERNAL_SERVER_ERROR); + } + } + + @PUT + @Path("/zk/put") + @Produces(MediaType.TEXT_PLAIN) + @ApiOperation(value = "Get content of the znode") + @ApiResponses(value = { // + @ApiResponse(code = 200, message = "Success"), // + @ApiResponse(code = 404, message = "ZK Path not found"), // + @ApiResponse(code = 204, message = "No Content"), // + @ApiResponse(code = 500, message = "Internal server error")}) + public SuccessResponse putData( + @ApiParam(value = "Zookeeper Path, must start with /", required = true, defaultValue = "/") @QueryParam("path") @DefaultValue("") String path, + @ApiParam(value = "Content", required = true) @QueryParam("data") @DefaultValue("") String content, + @ApiParam(value = "expectedVersion", required = true, defaultValue = "-1") @QueryParam("expectedVersion") @DefaultValue("-1") String expectedVersion, + @ApiParam(value = "accessOption", required = true, defaultValue = "1") @QueryParam("accessOption") @DefaultValue("1") String accessOption) { + path = validateAndNormalizeZKPath(path); + ZNRecord record = null; + if (content != null) { + record = (ZNRecord) _znRecordSerializer.deserialize(content.getBytes(Charsets.UTF_8)); + } + try { + boolean result = pinotHelixResourceManager Review comment: Will this allow any arbitrary path to be set with any arbitrary value (as-in possibly corrupt the cluster state)? ########## File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/ZookeeperResource.java ########## @@ -75,6 +80,63 @@ public String getData( return null; } + @DELETE + @Path("/zk/delete") Review comment: The `delete` api seems a bit dangerous. Should we protect it to only allow deleting certain paths? Seems this api will allow deleting the entire cluster? ########## File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/ZookeeperResource.java ########## @@ -75,6 +80,63 @@ public String getData( return null; } + @DELETE + @Path("/zk/delete") + @Produces(MediaType.TEXT_PLAIN) + @ApiOperation(value = "Get content of the znode") Review comment: `value` should be `delete` and not `Get`? ---------------------------------------------------------------- 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: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org