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

Reply via email to