suvodeep-pyne commented on a change in pull request #5808: URL: https://github.com/apache/incubator-pinot/pull/5808#discussion_r466602375
########## File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/dashboard/ThirdEyeDashboardApplication.java ########## @@ -139,36 +114,11 @@ public void run(ThirdEyeDashboardConfiguration config, Environment env) final JerseyEnvironment jersey = env.jersey(); injector = Guice.createInjector(new ThirdEyeDashboardModule(config, env, DAO_REGISTRY)); Stream.of( - DetectionConfigurationResource.class, - DatasetAutoOnboardResource.class, - DashboardResource.class, - CacheResource.class, AnomalyResource.class, - EntityManagerResource.class, - MetricConfigResource.class, - DatasetConfigResource.class, - AdminResource.class, - SummaryResource.class, - ThirdEyeResource.class, - DataResource.class, - AnomaliesResource.class, - EntityMappingResource.class, - OnboardDatasetMetricResource.class, - AutoOnboardResource.class, - ConfigResource.class, - CustomizedEventResource.class, - AnomalyFlattenResource.class, - UserDashboardResource.class, - ApplicationResource.class, - DetectionResource.class, - DetectionAlertResource.class, - YamlResource.class, - SqlDataSourceResource.class, - AlertResource.class, - RootCauseTemplateResource.class, - RootCauseSessionResource.class, RootCauseMetricResource.class, - AnomalySearchResource.class + RootCauseSessionResource.class, + RootCauseTemplateResource.class, Review comment: There are RCA classes and they need further refactor themselves which seemed a bit out of scope. For example, `RootCauseResource` initialization is conditional on the config but `RootCauseSessionResource` and `RootCauseTemplateResource` are not! Also, all of them contribute to the "/rootcause" namespace. I think this needs to be investigated and solved separately. Another example is `AnomalyResource` which maps to "/dashboard". Generally by convention, path namespaces and class names are kept very similar so as to understand context. Here, there is a DashboardResource and AnomalyResource both pointing to "/dashboard" with a different set of methods. This again seems a separate PR to me. I want to fix them but not in this PR. Currently, the changes in this PR are mostly limited to moving "@Path" from class level to method level. ---------------------------------------------------------------- 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