You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2020/05/23 00:42:49 UTC
[GitHub] [incubator-pinot] vincentchenjl opened a new pull request #5435: [TE] clean up decprecated/unused code
vincentchenjl opened a new pull request #5435:
URL: https://github.com/apache/incubator-pinot/pull/5435
- Clean up deprecated code
- Refactor a bit to remove class dependency
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] xiaohui-sun commented on pull request #5435: [TE] clean up decprecated/unused code
Posted by GitBox <gi...@apache.org>.
xiaohui-sun commented on pull request #5435:
URL: https://github.com/apache/incubator-pinot/pull/5435#issuecomment-634438517
@cecilynie @cyenjung FYI.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] xiaohui-sun commented on a change in pull request #5435: [TE] clean up decprecated/unused code
Posted by GitBox <gi...@apache.org>.
xiaohui-sun commented on a change in pull request #5435:
URL: https://github.com/apache/incubator-pinot/pull/5435#discussion_r430861217
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/anomaly/task/TaskConstants.java
##########
@@ -26,15 +26,8 @@
DETECTION,
DETECTION_ALERT,
YAML_DETECTION_ONBOARD,
- ANOMALY_DETECTION,
- MERGE,
// TODO: deprecate ALERT task type
Review comment:
Remove //TODO
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/common/BaseThirdEyeApplication.java
##########
@@ -51,11 +46,9 @@
protected final Logger LOG = LoggerFactory.getLogger(this.getClass());
protected AnomalyFunctionManager anomalyFunctionDAO;
Review comment:
anomalyFunctionDAO is not used, including AnomalyFunctionManager.
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/dashboard/resources/AnomalyResource.java
##########
@@ -101,21 +99,18 @@
private MergedAnomalyResultManager anomalyMergedResultDAO;
private AlertConfigManager emailConfigurationDAO;
private MergedAnomalyResultManager mergedAnomalyResultDAO;
- private AutotuneConfigManager autotuneConfigDAO;
private DatasetConfigManager datasetConfigDAO;
private AnomalyFunctionFactory anomalyFunctionFactory;
private AlertFilterFactory alertFilterFactory;
Review comment:
Not used.
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/dashboard/ThirdEyeDashboardApplication.java
##########
@@ -160,14 +155,12 @@ public void run(ThirdEyeDashboardConfiguration config, Environment env)
AnomalyFunctionFactory anomalyFunctionFactory = new AnomalyFunctionFactory(config.getFunctionConfigPath());
Review comment:
AnomalyFunctionFactory is not used.
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/dashboard/resources/AnomalyResource.java
##########
@@ -101,21 +99,18 @@
private MergedAnomalyResultManager anomalyMergedResultDAO;
private AlertConfigManager emailConfigurationDAO;
private MergedAnomalyResultManager mergedAnomalyResultDAO;
- private AutotuneConfigManager autotuneConfigDAO;
private DatasetConfigManager datasetConfigDAO;
private AnomalyFunctionFactory anomalyFunctionFactory;
private AlertFilterFactory alertFilterFactory;
private LoadingCache<String, Long> collectionMaxDataTimeCache;
private static final DAORegistry DAO_REGISTRY = DAORegistry.getInstance();
- public AnomalyResource(AnomalyFunctionFactory anomalyFunctionFactory, AlertFilterFactory alertFilterFactory,
- AlertFilterAutotuneFactory alertFilterAutotuneFactory) {
+ public AnomalyResource(AnomalyFunctionFactory anomalyFunctionFactory, AlertFilterFactory alertFilterFactory) {
this.anomalyFunctionDAO = DAO_REGISTRY.getAnomalyFunctionDAO();
this.anomalyMergedResultDAO = DAO_REGISTRY.getMergedAnomalyResultDAO();
this.emailConfigurationDAO = DAO_REGISTRY.getAlertConfigDAO();
this.mergedAnomalyResultDAO = DAO_REGISTRY.getMergedAnomalyResultDAO();
- this.autotuneConfigDAO = DAO_REGISTRY.getAutotuneConfigDAO();
this.datasetConfigDAO = DAO_REGISTRY.getDatasetConfigDAO();
this.anomalyFunctionFactory = anomalyFunctionFactory;
Review comment:
This is not used.
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/datalayer/pojo/DatasetConfigBean.java
##########
@@ -93,10 +89,6 @@
private boolean requiresCompletenessCheck = false;
Review comment:
Not used.
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/anomaly/task/TaskInfoFactory.java
##########
@@ -58,25 +54,9 @@ public static TaskInfo getTaskInfoFromTaskType(TaskType taskType, String taskInf
case YAML_DETECTION_ONBOARD:
taskInfo = OBJECT_MAPPER.readValue(taskInfoString, YamlOnboardingTaskInfo.class);
break;
- case ANOMALY_DETECTION:
- taskInfo = OBJECT_MAPPER.readValue(taskInfoString, DetectionTaskInfo.class);
- break;
- case MERGE:
- LOG.error("TaskType MERGE not supported");
- break;
case MONITOR:
taskInfo = OBJECT_MAPPER.readValue(taskInfoString, MonitorTaskInfo.class);
break;
- case ALERT:
- case ALERT2:
- taskInfo = OBJECT_MAPPER.readValue(taskInfoString, AlertTaskInfo.class);
- break;
- case DATA_COMPLETENESS:
- taskInfo = OBJECT_MAPPER.readValue(taskInfoString, DataCompletenessTaskInfo.class);
- break;
- case CLASSIFICATION:
- taskInfo = OBJECT_MAPPER.readValue(taskInfoString, ClassificationTaskInfo.class);
- break;
default:
LOG.error("TaskType must be one of ANOMALY_DETECTION, MONITOR, ALERT, ALERT2, DATA_COMPLETENESS, CLASSIFICATION");
Review comment:
Update error log accordingly.
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/dashboard/resources/AnomalyResource.java
##########
@@ -101,21 +99,18 @@
private MergedAnomalyResultManager anomalyMergedResultDAO;
private AlertConfigManager emailConfigurationDAO;
private MergedAnomalyResultManager mergedAnomalyResultDAO;
- private AutotuneConfigManager autotuneConfigDAO;
private DatasetConfigManager datasetConfigDAO;
private AnomalyFunctionFactory anomalyFunctionFactory;
private AlertFilterFactory alertFilterFactory;
private LoadingCache<String, Long> collectionMaxDataTimeCache;
private static final DAORegistry DAO_REGISTRY = DAORegistry.getInstance();
- public AnomalyResource(AnomalyFunctionFactory anomalyFunctionFactory, AlertFilterFactory alertFilterFactory,
- AlertFilterAutotuneFactory alertFilterAutotuneFactory) {
+ public AnomalyResource(AnomalyFunctionFactory anomalyFunctionFactory, AlertFilterFactory alertFilterFactory) {
this.anomalyFunctionDAO = DAO_REGISTRY.getAnomalyFunctionDAO();
Review comment:
This is not used.
##########
File path: thirdeye/thirdeye-pinot/src/test/java/org/apache/pinot/thirdeye/datalayer/DaoTestUtils.java
##########
@@ -275,22 +273,6 @@ public static DetectionStatusDTO getTestDetectionStatus(String dataset, long dat
return detectionStatusDTO;
}
- public static AutotuneConfigDTO getTestAutotuneConfig(long functionId, long start, long end) {
- AutotuneConfigDTO autotuneConfigDTO = new AutotuneConfigDTO();
- autotuneConfigDTO.setFunctionId(functionId);
- autotuneConfigDTO.setStartTime(start);
- autotuneConfigDTO.setEndTime(end);
- autotuneConfigDTO.setPerformanceEvaluationMethod(PerformanceEvaluationMethod.ANOMALY_PERCENTAGE);
- autotuneConfigDTO.setLastUpdateTimestamp(DateTime.now().getMillis());
- Map<String, String> config = new HashMap<>();
- config.put("ConfigKey", "ConfigValue");
- autotuneConfigDTO.setConfiguration(config);
- Map<String, Double> performance = new HashMap<>();
- performance.put(autotuneConfigDTO.getPerformanceEvaluationMethod().name(), 0.5);
- autotuneConfigDTO.setPerformance(performance);
- return autotuneConfigDTO;
- }
-
public static ClassificationConfigDTO getTestClassificationConfig(String name, List<Long> mainFunctionIdList,
Review comment:
Not used.
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/dashboard/resources/AnomalyResource.java
##########
@@ -101,21 +99,18 @@
private MergedAnomalyResultManager anomalyMergedResultDAO;
private AlertConfigManager emailConfigurationDAO;
private MergedAnomalyResultManager mergedAnomalyResultDAO;
- private AutotuneConfigManager autotuneConfigDAO;
private DatasetConfigManager datasetConfigDAO;
private AnomalyFunctionFactory anomalyFunctionFactory;
private AlertFilterFactory alertFilterFactory;
private LoadingCache<String, Long> collectionMaxDataTimeCache;
private static final DAORegistry DAO_REGISTRY = DAORegistry.getInstance();
- public AnomalyResource(AnomalyFunctionFactory anomalyFunctionFactory, AlertFilterFactory alertFilterFactory,
- AlertFilterAutotuneFactory alertFilterAutotuneFactory) {
+ public AnomalyResource(AnomalyFunctionFactory anomalyFunctionFactory, AlertFilterFactory alertFilterFactory) {
this.anomalyFunctionDAO = DAO_REGISTRY.getAnomalyFunctionDAO();
this.anomalyMergedResultDAO = DAO_REGISTRY.getMergedAnomalyResultDAO();
Review comment:
What is the difference between anomalyMergedResultDAO and mergedAnomalyResultDAO. Seems duplicate.
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/dashboard/ThirdEyeDashboardApplication.java
##########
@@ -160,14 +155,12 @@ public void run(ThirdEyeDashboardConfiguration config, Environment env)
AnomalyFunctionFactory anomalyFunctionFactory = new AnomalyFunctionFactory(config.getFunctionConfigPath());
AlertFilterFactory alertFilterFactory = new AlertFilterFactory(config.getAlertFilterConfigPath());
Review comment:
AlertFilterFactory is not used.
All the classes under org.apache.pinot.thirdeye.detector.email.filter are not used.
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/dashboard/ThirdEyeDashboardApplication.java
##########
@@ -181,7 +174,6 @@ public void run(ThirdEyeDashboardConfiguration config, Environment env)
env.jersey().register(new AutoOnboardResource(config));
env.jersey().register(new ConfigResource(DAO_REGISTRY.getConfigDAO()));
env.jersey().register(new CustomizedEventResource(DAO_REGISTRY.getEventDAO()));
- env.jersey().register(new TimeSeriesResource());
Review comment:
The above three lines are not used.
env.jersey().register(new DataResource(anomalyFunctionFactory, alertFilterFactory)); env.jersey().register(new DataResource(anomalyFunctionFactory, alertFilterFactory));
env.jersey().register(new AnomaliesResource(anomalyFunctionFactory, alertFilterFactory));
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/anomaly/task/TaskConstants.java
##########
@@ -26,15 +26,8 @@
DETECTION,
DETECTION_ALERT,
YAML_DETECTION_ONBOARD,
- ANOMALY_DETECTION,
- MERGE,
// TODO: deprecate ALERT task type
- ALERT,
- ALERT2,
- MONITOR,
- DATA_COMPLETENESS,
- CLASSIFICATION,
- REPLAY
+ MONITOR
Review comment:
Add a simple description for each task type.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] vincentchenjl commented on a change in pull request #5435: [TE] clean up decprecated/unused code
Posted by GitBox <gi...@apache.org>.
vincentchenjl commented on a change in pull request #5435:
URL: https://github.com/apache/incubator-pinot/pull/5435#discussion_r431526938
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/dashboard/ThirdEyeDashboardApplication.java
##########
@@ -160,14 +155,12 @@ public void run(ThirdEyeDashboardConfiguration config, Environment env)
AnomalyFunctionFactory anomalyFunctionFactory = new AnomalyFunctionFactory(config.getFunctionConfigPath());
Review comment:
`anomalyFunctionFactory` is used to initialize `AnomalyResource`, `DataResource`, and `AnomaliesResource`. Could we confirm all these three resources are not used?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] vincentchenjl commented on a change in pull request #5435: [TE] clean up decprecated/unused code
Posted by GitBox <gi...@apache.org>.
vincentchenjl commented on a change in pull request #5435:
URL: https://github.com/apache/incubator-pinot/pull/5435#discussion_r431522638
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/anomaly/task/TaskConstants.java
##########
@@ -26,15 +26,8 @@
DETECTION,
DETECTION_ALERT,
YAML_DETECTION_ONBOARD,
- ANOMALY_DETECTION,
- MERGE,
// TODO: deprecate ALERT task type
- ALERT,
- ALERT2,
- MONITOR,
- DATA_COMPLETENESS,
- CLASSIFICATION,
- REPLAY
+ MONITOR
Review comment:
Added one line description each type.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] xiaohui-sun merged pull request #5435: [TE] clean up decprecated/unused code
Posted by GitBox <gi...@apache.org>.
xiaohui-sun merged pull request #5435:
URL: https://github.com/apache/incubator-pinot/pull/5435
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] vincentchenjl commented on a change in pull request #5435: [TE] clean up decprecated/unused code
Posted by GitBox <gi...@apache.org>.
vincentchenjl commented on a change in pull request #5435:
URL: https://github.com/apache/incubator-pinot/pull/5435#discussion_r431529266
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/dashboard/resources/AnomalyResource.java
##########
@@ -101,21 +99,18 @@
private MergedAnomalyResultManager anomalyMergedResultDAO;
private AlertConfigManager emailConfigurationDAO;
private MergedAnomalyResultManager mergedAnomalyResultDAO;
- private AutotuneConfigManager autotuneConfigDAO;
private DatasetConfigManager datasetConfigDAO;
private AnomalyFunctionFactory anomalyFunctionFactory;
private AlertFilterFactory alertFilterFactory;
private LoadingCache<String, Long> collectionMaxDataTimeCache;
private static final DAORegistry DAO_REGISTRY = DAORegistry.getInstance();
- public AnomalyResource(AnomalyFunctionFactory anomalyFunctionFactory, AlertFilterFactory alertFilterFactory,
- AlertFilterAutotuneFactory alertFilterAutotuneFactory) {
+ public AnomalyResource(AnomalyFunctionFactory anomalyFunctionFactory, AlertFilterFactory alertFilterFactory) {
this.anomalyFunctionDAO = DAO_REGISTRY.getAnomalyFunctionDAO();
Review comment:
`anomalyFunctionDAO` is used in multiple places in this class.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] xiaohui-sun commented on a change in pull request #5435: [TE] clean up decprecated/unused code
Posted by GitBox <gi...@apache.org>.
xiaohui-sun commented on a change in pull request #5435:
URL: https://github.com/apache/incubator-pinot/pull/5435#discussion_r429602883
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/anomaly/ThirdEyeAnomalyApplication.java
##########
@@ -273,9 +224,4 @@ private void initAnomalyClassifierFactory(String anomalyClassifierConfigPath) {
}
Review comment:
initAnomalyClassifierFactory, initAlertFilterFactory could be deleted.
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/anomaly/task/TaskInfoFactory.java
##########
@@ -67,16 +64,6 @@ public static TaskInfo getTaskInfoFromTaskType(TaskType taskType, String taskInf
case MONITOR:
taskInfo = OBJECT_MAPPER.readValue(taskInfoString, MonitorTaskInfo.class);
break;
- case ALERT:
Review comment:
ANOMALY_DETECTION and MERGE tasks are not used.
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/anomaly/ThirdEyeAnomalyApplication.java
##########
@@ -61,18 +54,12 @@
public class ThirdEyeAnomalyApplication
extends BaseThirdEyeApplication<ThirdEyeAnomalyConfiguration> {
- private DetectionJobScheduler detectionJobScheduler = null;
private TaskDriver taskDriver = null;
private MonitorJobScheduler monitorJobScheduler = null;
- private AlertJobSchedulerV2 alertJobSchedulerV2;
private AnomalyFunctionFactory anomalyFunctionFactory = null;
private AutoOnboardService autoOnboardService = null;
- private DataCompletenessScheduler dataCompletenessScheduler = null;
private AlertFilterFactory alertFilterFactory = null;
private AnomalyClassifierFactory anomalyClassifierFactory = null;
Review comment:
anomalyClassifierFactory not used.
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/anomaly/ThirdEyeAnomalyApplication.java
##########
@@ -61,18 +54,12 @@
public class ThirdEyeAnomalyApplication
extends BaseThirdEyeApplication<ThirdEyeAnomalyConfiguration> {
- private DetectionJobScheduler detectionJobScheduler = null;
private TaskDriver taskDriver = null;
private MonitorJobScheduler monitorJobScheduler = null;
- private AlertJobSchedulerV2 alertJobSchedulerV2;
private AnomalyFunctionFactory anomalyFunctionFactory = null;
private AutoOnboardService autoOnboardService = null;
- private DataCompletenessScheduler dataCompletenessScheduler = null;
private AlertFilterFactory alertFilterFactory = null;
Review comment:
alertFilterFactory not used.
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/anomaly/task/TaskRunnerFactory.java
##########
@@ -59,16 +56,6 @@ public static TaskRunner getTaskRunnerFromTaskType(TaskType taskType) {
case MONITOR:
taskRunner = new MonitorTaskRunner();
break;
- case DATA_COMPLETENESS:
Review comment:
ANOMALY_DETECTION and MERGE tasks are not used.
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/completeness/checker/Wo4WAvgDataCompletenessAlgorithm.java
##########
@@ -41,6 +41,7 @@
/**
* This is the implementation of the WO4W Average function or checking data completeness of datasets
*/
+@Deprecated
public class Wo4WAvgDataCompletenessAlgorithm implements DataCompletenessAlgorithm {
public static double DEFAULT_EXPECTED_COMPLETENESS = 80;
Review comment:
This class could be deleted.
Let's delete everything under /completeness.checker
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/completeness/checker/DataCompletenessUtils.java
##########
@@ -261,13 +261,4 @@ public static DateTimeFormatter getDateTimeFormatterForDataset(TimeSpec timeSpec
return bucketNameToCountStar;
}
- public static double getPercentCompleteness(PercentCompletenessFunctionInput input) {
Review comment:
This whole class could be deleted.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] vincentchenjl commented on a change in pull request #5435: [TE] clean up decprecated/unused code
Posted by GitBox <gi...@apache.org>.
vincentchenjl commented on a change in pull request #5435:
URL: https://github.com/apache/incubator-pinot/pull/5435#discussion_r431528040
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/dashboard/resources/AnomalyResource.java
##########
@@ -101,21 +99,18 @@
private MergedAnomalyResultManager anomalyMergedResultDAO;
private AlertConfigManager emailConfigurationDAO;
private MergedAnomalyResultManager mergedAnomalyResultDAO;
- private AutotuneConfigManager autotuneConfigDAO;
private DatasetConfigManager datasetConfigDAO;
private AnomalyFunctionFactory anomalyFunctionFactory;
private AlertFilterFactory alertFilterFactory;
private LoadingCache<String, Long> collectionMaxDataTimeCache;
private static final DAORegistry DAO_REGISTRY = DAORegistry.getInstance();
- public AnomalyResource(AnomalyFunctionFactory anomalyFunctionFactory, AlertFilterFactory alertFilterFactory,
- AlertFilterAutotuneFactory alertFilterAutotuneFactory) {
+ public AnomalyResource(AnomalyFunctionFactory anomalyFunctionFactory, AlertFilterFactory alertFilterFactory) {
this.anomalyFunctionDAO = DAO_REGISTRY.getAnomalyFunctionDAO();
this.anomalyMergedResultDAO = DAO_REGISTRY.getMergedAnomalyResultDAO();
Review comment:
Both these two are used in the class. If we r sure `AnomalyResource` is not used, we can remove the whole class.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] jihaozh commented on a change in pull request #5435: [TE] clean up decprecated/unused code
Posted by GitBox <gi...@apache.org>.
jihaozh commented on a change in pull request #5435:
URL: https://github.com/apache/incubator-pinot/pull/5435#discussion_r432004017
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/dashboard/ThirdEyeDashboardApplication.java
##########
@@ -160,14 +155,12 @@ public void run(ThirdEyeDashboardConfiguration config, Environment env)
AnomalyFunctionFactory anomalyFunctionFactory = new AnomalyFunctionFactory(config.getFunctionConfigPath());
Review comment:
I think `AnomalyResource` is not used. In `AnomaliesResource`, only the search endpoints listed by Harley is used. `DataResource` is being used, but it shouldn't need `anomalyFunctionFactory` anymore.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] vincentchenjl commented on a change in pull request #5435: [TE] clean up decprecated/unused code
Posted by GitBox <gi...@apache.org>.
vincentchenjl commented on a change in pull request #5435:
URL: https://github.com/apache/incubator-pinot/pull/5435#discussion_r432789651
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/dashboard/ThirdEyeDashboardApplication.java
##########
@@ -160,14 +155,12 @@ public void run(ThirdEyeDashboardConfiguration config, Environment env)
AnomalyFunctionFactory anomalyFunctionFactory = new AnomalyFunctionFactory(config.getFunctionConfigPath());
Review comment:
When we search for anomalies by using `/anomalies/search/time/`, we construct `AnomalyDetails`, and it requires `BaseAnomalyFunction` to do it.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] vincentchenjl commented on a change in pull request #5435: [TE] clean up decprecated/unused code
Posted by GitBox <gi...@apache.org>.
vincentchenjl commented on a change in pull request #5435:
URL: https://github.com/apache/incubator-pinot/pull/5435#discussion_r430689663
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/anomaly/task/TaskRunnerFactory.java
##########
@@ -59,16 +56,6 @@ public static TaskRunner getTaskRunnerFromTaskType(TaskType taskType) {
case MONITOR:
taskRunner = new MonitorTaskRunner();
break;
- case DATA_COMPLETENESS:
Review comment:
Removed.
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/completeness/checker/Wo4WAvgDataCompletenessAlgorithm.java
##########
@@ -41,6 +41,7 @@
/**
* This is the implementation of the WO4W Average function or checking data completeness of datasets
*/
+@Deprecated
public class Wo4WAvgDataCompletenessAlgorithm implements DataCompletenessAlgorithm {
public static double DEFAULT_EXPECTED_COMPLETENESS = 80;
Review comment:
The issue is that `Wo4WAvgDataCompletenessAlgorithm` is used as default value for `DatasetConfigBean.dataCompletenessAlgorithm`. I understand that it is not used, but i am not sure about the impact if i remove this field from `DatasetConfigBean` and still have this field inside the JSON in DB.
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/completeness/checker/DataCompletenessUtils.java
##########
@@ -261,13 +261,4 @@ public static DateTimeFormatter getDateTimeFormatterForDataset(TimeSpec timeSpec
return bucketNameToCountStar;
}
- public static double getPercentCompleteness(PercentCompletenessFunctionInput input) {
Review comment:
This class is used in `Wo4WAvgDataCompletenessAlgorithm`. We need to drop both of them if we want to drop this. `Wo4WAvgDataCompletenessAlgorithm` cannot be dropped due to the issue below.
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/completeness/checker/DataCompletenessUtils.java
##########
@@ -261,13 +261,4 @@ public static DateTimeFormatter getDateTimeFormatterForDataset(TimeSpec timeSpec
return bucketNameToCountStar;
}
- public static double getPercentCompleteness(PercentCompletenessFunctionInput input) {
Review comment:
I remove `Wo4WAvgDataCompletenessAlgorithm` completely and also the field `DatasetConfigBean.dataCompletenessAlgorithm` from `DatasetConfigBean`.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] xiaohui-sun commented on a change in pull request #5435: [TE] clean up decprecated/unused code
Posted by GitBox <gi...@apache.org>.
xiaohui-sun commented on a change in pull request #5435:
URL: https://github.com/apache/incubator-pinot/pull/5435#discussion_r431935559
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/dashboard/resources/AnomalyResource.java
##########
@@ -101,21 +99,18 @@
private MergedAnomalyResultManager anomalyMergedResultDAO;
private AlertConfigManager emailConfigurationDAO;
private MergedAnomalyResultManager mergedAnomalyResultDAO;
- private AutotuneConfigManager autotuneConfigDAO;
private DatasetConfigManager datasetConfigDAO;
private AnomalyFunctionFactory anomalyFunctionFactory;
private AlertFilterFactory alertFilterFactory;
private LoadingCache<String, Long> collectionMaxDataTimeCache;
private static final DAORegistry DAO_REGISTRY = DAORegistry.getInstance();
- public AnomalyResource(AnomalyFunctionFactory anomalyFunctionFactory, AlertFilterFactory alertFilterFactory,
- AlertFilterAutotuneFactory alertFilterAutotuneFactory) {
+ public AnomalyResource(AnomalyFunctionFactory anomalyFunctionFactory, AlertFilterFactory alertFilterFactory) {
this.anomalyFunctionDAO = DAO_REGISTRY.getAnomalyFunctionDAO();
this.anomalyMergedResultDAO = DAO_REGISTRY.getMergedAnomalyResultDAO();
Review comment:
The question is duplication. We should remove one of them.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] vincentchenjl commented on a change in pull request #5435: [TE] clean up decprecated/unused code
Posted by GitBox <gi...@apache.org>.
vincentchenjl commented on a change in pull request #5435:
URL: https://github.com/apache/incubator-pinot/pull/5435#discussion_r432790440
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/dashboard/resources/AnomalyResource.java
##########
@@ -101,21 +99,18 @@
private MergedAnomalyResultManager anomalyMergedResultDAO;
private AlertConfigManager emailConfigurationDAO;
private MergedAnomalyResultManager mergedAnomalyResultDAO;
- private AutotuneConfigManager autotuneConfigDAO;
private DatasetConfigManager datasetConfigDAO;
private AnomalyFunctionFactory anomalyFunctionFactory;
private AlertFilterFactory alertFilterFactory;
Review comment:
It is used in line 141.
`anomalyResults = AlertFilterHelper.applyFiltrationRule(anomalyResults, alertFilterFactory);`
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] vincentchenjl commented on a change in pull request #5435: [TE] clean up decprecated/unused code
Posted by GitBox <gi...@apache.org>.
vincentchenjl commented on a change in pull request #5435:
URL: https://github.com/apache/incubator-pinot/pull/5435#discussion_r431527731
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/dashboard/ThirdEyeDashboardApplication.java
##########
@@ -160,14 +155,12 @@ public void run(ThirdEyeDashboardConfiguration config, Environment env)
AnomalyFunctionFactory anomalyFunctionFactory = new AnomalyFunctionFactory(config.getFunctionConfigPath());
AlertFilterFactory alertFilterFactory = new AlertFilterFactory(config.getAlertFilterConfigPath());
Review comment:
Same as above.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] xiaohui-sun commented on a change in pull request #5435: [TE] clean up decprecated/unused code
Posted by GitBox <gi...@apache.org>.
xiaohui-sun commented on a change in pull request #5435:
URL: https://github.com/apache/incubator-pinot/pull/5435#discussion_r431942838
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/dashboard/ThirdEyeDashboardApplication.java
##########
@@ -160,14 +155,12 @@ public void run(ThirdEyeDashboardConfiguration config, Environment env)
AnomalyFunctionFactory anomalyFunctionFactory = new AnomalyFunctionFactory(config.getFunctionConfigPath());
Review comment:
@jihaozh can you check which resource is used? I believe most endpoints in AnomalyResource and AnomaliesResource are not used. From Harley's doc only "/anomalies/search/time/" is used, but let's double check with them.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] vincentchenjl commented on a change in pull request #5435: [TE] clean up decprecated/unused code
Posted by GitBox <gi...@apache.org>.
vincentchenjl commented on a change in pull request #5435:
URL: https://github.com/apache/incubator-pinot/pull/5435#discussion_r431528717
##########
File path: thirdeye/thirdeye-pinot/src/test/java/org/apache/pinot/thirdeye/datalayer/DaoTestUtils.java
##########
@@ -275,22 +273,6 @@ public static DetectionStatusDTO getTestDetectionStatus(String dataset, long dat
return detectionStatusDTO;
}
- public static AutotuneConfigDTO getTestAutotuneConfig(long functionId, long start, long end) {
- AutotuneConfigDTO autotuneConfigDTO = new AutotuneConfigDTO();
- autotuneConfigDTO.setFunctionId(functionId);
- autotuneConfigDTO.setStartTime(start);
- autotuneConfigDTO.setEndTime(end);
- autotuneConfigDTO.setPerformanceEvaluationMethod(PerformanceEvaluationMethod.ANOMALY_PERCENTAGE);
- autotuneConfigDTO.setLastUpdateTimestamp(DateTime.now().getMillis());
- Map<String, String> config = new HashMap<>();
- config.put("ConfigKey", "ConfigValue");
- autotuneConfigDTO.setConfiguration(config);
- Map<String, Double> performance = new HashMap<>();
- performance.put(autotuneConfigDTO.getPerformanceEvaluationMethod().name(), 0.5);
- autotuneConfigDTO.setPerformance(performance);
- return autotuneConfigDTO;
- }
-
public static ClassificationConfigDTO getTestClassificationConfig(String name, List<Long> mainFunctionIdList,
Review comment:
This can be removed in next PR with all the unused DAO objects.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] xiaohui-sun commented on a change in pull request #5435: [TE] clean up decprecated/unused code
Posted by GitBox <gi...@apache.org>.
xiaohui-sun commented on a change in pull request #5435:
URL: https://github.com/apache/incubator-pinot/pull/5435#discussion_r430698991
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/completeness/checker/DataCompletenessUtils.java
##########
@@ -261,13 +261,4 @@ public static DateTimeFormatter getDateTimeFormatterForDataset(TimeSpec timeSpec
return bucketNameToCountStar;
}
- public static double getPercentCompleteness(PercentCompletenessFunctionInput input) {
Review comment:
We should remove it and also remove getter/setter.
That should be safe.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org