suvodeep-pyne commented on pull request #5808: URL: https://github.com/apache/incubator-pinot/pull/5808#issuecomment-671486383
Thanks @vincentchenjl The core idea of this refactor is to have a coherence between the url path and the Resource class. Why? Because the goal of Jersey Resource class is provide a binding from a url to a java method. So for example, `RootResource` implies `/`, `ThirdEyeResource` inside `RootResource` implies the path `/thirdeye`, etc. That way you have a clear hierarchy in the code as to how the urls are set up. There are still anomalies in the code like `DetectionAlertResource` which is hosted at `/groups` which is really difficult to correlate. Grouping business logic can be done in 2 ways. - Documentation level: Use Swagger tags to group a set of APIs together. We already do this. - Code Level: Have core logic classes which hold the business logic instead of the Resource classes. This resources can delegate calls to these core logic classes. I agree with your idea to have a way of grouping things into bins like detection, RCA, etc. We should do this in our business logic. However, keeping all the business logic in Jersey Resource classes is not the best approach as the codebase grows. Also, it gets messy when you need to share logic in one Resource with another. Let me know what you think. ---------------------------------------------------------------- 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
