You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pinot.apache.org by GitBox <gi...@apache.org> on 2018/12/11 01:25:10 UTC

[GitHub] jihaozh commented on a change in pull request #3603: [TE] Endpoints for create and edit alert yaml along with validators

jihaozh commented on a change in pull request #3603: [TE] Endpoints for create and edit alert yaml along with validators
URL: https://github.com/apache/incubator-pinot/pull/3603#discussion_r240444195
 
 

 ##########
 File path: thirdeye/thirdeye-pinot/src/main/java/com/linkedin/thirdeye/detection/yaml/YamlResource.java
 ##########
 @@ -205,45 +237,105 @@ private void validatePipeline(DetectionConfigDTO detectionConfig) throws Excepti
     detectionConfig.setId(id);
   }
 
-  /**
-   translate alert yaml to detection alert config
-   */
-  private DetectionAlertConfigDTO getDetectionAlertConfig(Map<String, Object> alertYaml, Long detectionConfigId) {
-    Preconditions.checkArgument(alertYaml.containsKey(PROP_NAME), "alert name missing");
+  @SuppressWarnings("unchecked")
+  public DetectionAlertConfigDTO createDetectionAlertConfig(String yamlAlertConfig, Map<String, String> responseMessage ) {
+    if (!alertValidator.validateYAMLConfig(yamlAlertConfig, responseMessage)) {
+      return null;
+    }
 
-    // try to retrieve existing alert config
-    List<DetectionAlertConfigDTO> existingAlertConfigDTOs =
-        this.detectionAlertConfigDAO.findByPredicate(Predicate.EQ("name", MapUtils.getString(alertYaml, PROP_NAME)));
+    TreeMap<String, Object> newAlertConfigMap = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
+    newAlertConfigMap.putAll((Map<String, Object>) this.yaml.load(yamlAlertConfig));
 
-    if (existingAlertConfigDTOs.isEmpty()) {
-      // if alert does not exist, create a new alerter
-      return this.alertConfigTranslator.generateDetectionAlertConfig(alertYaml, Collections.singletonList(detectionConfigId), new HashMap<>());
-    } else {
-      // get existing detection alerter
-      DetectionAlertConfigDTO existingAlertConfigDTO = existingAlertConfigDTOs.get(0);
-      if (alertYaml.containsKey(PROP_TYPE)) {
-        // if alert Yaml contains alert configuration, update existing alert config properties
-        Set<Long> detectionConfigIds =
-            new HashSet(ConfigUtils.getLongs(existingAlertConfigDTO.getProperties().get(PROP_DETECTION_CONFIG_ID)));
-        detectionConfigIds.add(detectionConfigId);
-        DetectionAlertConfigDTO alertConfigDTO =
-            this.alertConfigTranslator.generateDetectionAlertConfig(alertYaml, detectionConfigIds, existingAlertConfigDTO.getVectorClocks());
-        alertConfigDTO.setId(existingAlertConfigDTO.getId());
-        alertConfigDTO.setHighWaterMark(existingAlertConfigDTO.getHighWaterMark());
-        return alertConfigDTO;
-      } else {
-        // Yaml does not contains alert config properties, add the detection pipeline to a existing alerter
-        Map<Long, Long> existingVectorClocks = existingAlertConfigDTO.getVectorClocks();
-        if (!existingVectorClocks.containsKey(detectionConfigId)) {
-          existingVectorClocks.put(detectionConfigId, 0L);
-        }
-        Set<Long> detectionConfigIds =
-            new HashSet(ConfigUtils.getList(existingAlertConfigDTO.getProperties().get(PROP_DETECTION_CONFIG_ID)));
-        detectionConfigIds.add(detectionConfigId);
-        existingAlertConfigDTO.getProperties().put(PROP_DETECTION_CONFIG_ID, detectionConfigIds);
-        return existingAlertConfigDTO;
+    // Check if a subscription group with the name already exists
+    String subsGroupName = MapUtils.getString(newAlertConfigMap, PROP_SUBS_GROUP_NAME);
+    if (StringUtils.isEmpty(subsGroupName)) {
+      responseMessage.put("message", "Subscription group name field cannot be left empty.");
+      return null;
+    }
+    List<DetectionAlertConfigDTO> alertConfigDTOS = this.detectionAlertConfigDAO
+        .findByPredicate(Predicate.EQ("name", MapUtils.getString(newAlertConfigMap, PROP_SUBS_GROUP_NAME)));
+    if (!alertConfigDTOS.isEmpty()) {
+      responseMessage.put("message", "Subscription group name is already taken. Please use a different name.");
+      return null;
+    }
+
+    // Translate config from YAML to detection alert config (JSON)
+    DetectionAlertConfigDTO alertConfig = this.alertConfigTranslator.translate(newAlertConfigMap);
+    alertConfig.setYaml(yamlAlertConfig);
+
+    // Validate the config before saving it
+    if (!alertValidator.validateConfig(alertConfig, responseMessage)) {
+      return null;
+    }
+
+    return alertConfig;
+  }
+
+  @POST
 
 Review comment:
   Probably use PUT for update action

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pinot.apache.org
For additional commands, e-mail: dev-help@pinot.apache.org