wirybeaver commented on code in PR #10758: URL: https://github.com/apache/pinot/pull/10758#discussion_r1253978437
########## pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java: ########## @@ -884,6 +865,52 @@ public SuccessResponse deleteSegments( } } + @DELETE + @Consumes(MediaType.APPLICATION_JSON) + @Produces(MediaType.APPLICATION_JSON) + @Path("/segments/{tableName}/select") Review Comment: From the perspective of safety, since it's a delete api, my hunt is to enforce users MUST input non-empty startTime and endTime, which will break the compatibility that delete all segment if no filter is provides. That's another reason to have different PATH. User would know they are doing a dangerous operation if they use `DELETE /segments/{tableName}` meanwhile `DELETE /segments/{tableName}/choose` has a safety guard even though they forgot to add the timestamp. I am OK to add filters into `DELETE /segments/{tableName}` and remove `DELETE /segments/{tableName}/choose` if you thought the lack of prevention of missing time window is not a major concern. -- 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