You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ak...@apache.org on 2019/02/08 19:42:34 UTC
[incubator-pinot] branch master updated: [TE] Validation info to
display on UI (#3809)
This is an automated email from the ASF dual-hosted git repository.
akshayrai09 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 2587998 [TE] Validation info to display on UI (#3809)
2587998 is described below
commit 258799804f09d34614e82e8a09ffcddc3d381177
Author: Akshay Rai <ak...@gmail.com>
AuthorDate: Fri Feb 8 11:42:25 2019 -0800
[TE] Validation info to display on UI (#3809)
---
.../validators/DetectionAlertConfigValidator.java | 1 -
.../thirdeye/detection/yaml/YamlResource.java | 67 ++++++++++++----------
.../thirdeye/detection/yaml/YamlResourceTest.java | 32 ++++++-----
3 files changed, 55 insertions(+), 45 deletions(-)
diff --git a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/validators/DetectionAlertConfigValidator.java b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/validators/DetectionAlertConfigValidator.java
index 8224671..07e83d2 100644
--- a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/validators/DetectionAlertConfigValidator.java
+++ b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/validators/DetectionAlertConfigValidator.java
@@ -26,7 +26,6 @@ import java.util.Map;
import org.apache.commons.lang.StringUtils;
import org.apache.pinot.thirdeye.datalayer.util.Predicate;
import org.apache.pinot.thirdeye.datasource.DAORegistry;
-import org.junit.Assert;
import static org.apache.pinot.thirdeye.detection.yaml.YamlDetectionAlertConfigTranslator.*;
diff --git a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/YamlResource.java b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/YamlResource.java
index 441d79b..e5dd378 100644
--- a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/YamlResource.java
+++ b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/YamlResource.java
@@ -62,6 +62,7 @@ import org.apache.pinot.thirdeye.datasource.loader.AggregationLoader;
import org.apache.pinot.thirdeye.datasource.loader.DefaultAggregationLoader;
import org.apache.pinot.thirdeye.datasource.loader.DefaultTimeSeriesLoader;
import org.apache.pinot.thirdeye.datasource.loader.TimeSeriesLoader;
+import org.apache.pinot.thirdeye.detection.ConfigUtils;
import org.apache.pinot.thirdeye.detection.DataProvider;
import org.apache.pinot.thirdeye.detection.DefaultDataProvider;
import org.apache.pinot.thirdeye.detection.DetectionPipeline;
@@ -79,8 +80,8 @@ public class YamlResource {
protected static final Logger LOG = LoggerFactory.getLogger(YamlResource.class);
private static ObjectMapper OBJECT_MAPPER = new ObjectMapper();
- public static final String PROP_DETECTION_NAME = "detectionName";
- public static final String PROP_SUBS_GROUP_NAME = "subscriptionGroupName";
+ private static final String PROP_DETECTION_NAME = "detectionName";
+ private static final String PROP_SUBS_GROUP_NAME = "subscriptionGroupName";
private final DetectionConfigManager detectionConfigDAO;
private final DetectionAlertConfigManager detectionAlertConfigDAO;
@@ -190,7 +191,7 @@ public class YamlResource {
Map<String, Object> detectionYamlConfig;
try {
- detectionYamlConfig = (Map<String, Object>) this.yaml.load(detectionYaml);
+ detectionYamlConfig = ConfigUtils.getMap(this.yaml.load(detectionYaml));
} catch (Exception e){
return Response.status(Response.Status.BAD_REQUEST).entity(ImmutableMap.of("message", "detection yaml parsing error, " + e.getMessage())).build();
}
@@ -221,7 +222,7 @@ public class YamlResource {
Map<String, Object> notificationYamlConfig;
try {
- notificationYamlConfig = (Map<String, Object>) this.yaml.load(notificationYaml);
+ notificationYamlConfig = ConfigUtils.getMap(this.yaml.load(notificationYaml));
} catch (Exception e){
return Response.status(Response.Status.BAD_REQUEST)
.entity(ImmutableMap.of("message", "notification yaml parsing error, " + e.getMessage())).build();
@@ -233,9 +234,9 @@ public class YamlResource {
.findByPredicate(Predicate.EQ("name", groupName));
Response response;
if (!alertConfigDTOS.isEmpty()) {
- response = updateDetectionAlertConfigApi(notificationYaml, alertConfigDTOS.get(0).getId());
+ response = updateSubscriptionGroupApi(notificationYaml, alertConfigDTOS.get(0).getId());
} else {
- response = createDetectionAlertConfigApi(notificationYaml);
+ response = createSubscriptionGroupApi(notificationYaml);
}
if (response.getStatusInfo() != Response.Status.OK) {
@@ -243,7 +244,7 @@ public class YamlResource {
this.detectionConfigDAO.deleteById(detectionConfigId);
return Response.serverError().entity(response.getEntity()).build();
}
- long alertId = Long.parseLong(((Map<String, String>) response.getEntity()).get("detectionAlertConfigId"));
+ long alertId = Long.parseLong(ConfigUtils.getMap(response.getEntity()).get("detectionAlertConfigId").toString());
return Response.ok().entity(ImmutableMap.of("detectionConfigId", detectionConfig.getId(), "detectionAlertConfigId", alertId)).build();
}
@@ -338,7 +339,7 @@ public class YamlResource {
}
@SuppressWarnings("unchecked")
- DetectionAlertConfigDTO createDetectionAlertConfig(String yamlAlertConfig) throws ValidationException {
+ long createSubscriptionGroup(String yamlAlertConfig) throws ValidationException {
notificationValidator.validateYAMLConfig(yamlAlertConfig);
// Translate config from YAML to detection alert config (JSON)
@@ -357,11 +358,16 @@ public class YamlResource {
// Validate the config
notificationValidator.validateConfig(alertConfig);
- return alertConfig;
+ // Save the detection alert config
+ Preconditions.checkNotNull(alertConfig);
+ Long id = this.detectionAlertConfigDAO.save(alertConfig);
+ Preconditions.checkNotNull(id);
+
+ return alertConfig.getId();
}
@SuppressWarnings("unchecked")
- DetectionAlertConfigDTO updateDetectionAlertConfig(long oldAlertConfigID, String yamlAlertConfig) throws ValidationException {
+ void updateSubscriptionGroup(long oldAlertConfigID, String yamlAlertConfig) throws ValidationException {
DetectionAlertConfigDTO oldAlertConfig = this.detectionAlertConfigDAO.findById(oldAlertConfigID);
if (oldAlertConfig == null) {
throw new RuntimeException("Cannot find subscription group " + oldAlertConfigID);
@@ -380,7 +386,11 @@ public class YamlResource {
// Validate before updating the config
notificationValidator.validateUpdatedConfig(updatedAlertConfig, oldAlertConfig);
- return updatedAlertConfig;
+ // Save the updated notification config
+ int detectionAlertConfigId = this.detectionAlertConfigDAO.update(updatedAlertConfig);
+ if (detectionAlertConfigId <= 0) {
+ throw new RuntimeException("Failed to update the detection alert config.");
+ }
}
/**
@@ -412,26 +422,26 @@ public class YamlResource {
@Consumes(MediaType.TEXT_PLAIN)
@ApiOperation("Create a notification group using a YAML config")
@SuppressWarnings("unchecked")
- public Response createDetectionAlertConfigApi(
+ public Response createSubscriptionGroupApi(
@ApiParam("payload") String yamlAlertConfig) {
Map<String, String> responseMessage = new HashMap<>();
- Long detectionAlertConfigId;
+ long detectionAlertConfigId;
try {
- DetectionAlertConfigDTO alertConfig = createDetectionAlertConfig(yamlAlertConfig);
- Preconditions.checkNotNull(alertConfig);
-
- detectionAlertConfigId = this.detectionAlertConfigDAO.save(alertConfig);
- Preconditions.checkNotNull(detectionAlertConfigId);
+ detectionAlertConfigId = createSubscriptionGroup(yamlAlertConfig);
+ } catch (ValidationException e) {
+ LOG.warn("Validation error while creating notification group with payload " + yamlAlertConfig, e);
+ responseMessage.put("message", "Validation Error! " + e.getMessage());
+ return Response.serverError().entity(responseMessage).build();
} catch (Exception e) {
LOG.error("Error creating notification group with payload " + yamlAlertConfig, e);
- responseMessage.put("message", "Failed to create the notification group.");
+ responseMessage.put("message", "Failed to create the notification group. Reach out to the ThirdEye team.");
responseMessage.put("more-info", "Error = " + e.getMessage());
return Response.serverError().entity(responseMessage).build();
}
LOG.info("Notification group created with id " + detectionAlertConfigId + " using payload " + yamlAlertConfig);
responseMessage.put("message", "The notification group was created successfully.");
- responseMessage.put("more-info", "Record saved with id " + detectionAlertConfigId);
+ responseMessage.put("detectionAlertConfigId", String.valueOf(detectionAlertConfigId));
return Response.ok().entity(responseMessage).build();
}
@@ -440,27 +450,26 @@ public class YamlResource {
@Produces(MediaType.APPLICATION_JSON)
@Consumes(MediaType.TEXT_PLAIN)
@ApiOperation("Edit a notification group using a YAML config")
- public Response updateDetectionAlertConfigApi(
+ public Response updateSubscriptionGroupApi(
@ApiParam("payload") String yamlAlertConfig,
@ApiParam("the detection alert config id to edit") @PathParam("id") long id) {
Map<String, String> responseMessage = new HashMap<>();
try {
- DetectionAlertConfigDTO updatedAlertConfig = updateDetectionAlertConfig(id, yamlAlertConfig);
- Preconditions.checkNotNull(updatedAlertConfig);
-
- int detectionAlertConfigId = this.detectionAlertConfigDAO.update(updatedAlertConfig);
- if (detectionAlertConfigId <= 0) {
- throw new RuntimeException("Failed to update the detection alert config.");
- }
+ updateSubscriptionGroup(id, yamlAlertConfig);
+ } catch (ValidationException e) {
+ LOG.warn("Validation error while updating notification group " + id + " with payload " + yamlAlertConfig, e);
+ responseMessage.put("message", "Validation Error! " + e.getMessage());
+ return Response.serverError().entity(responseMessage).build();
} catch (Exception e) {
LOG.error("Error updating notification group " + id + " with payload " + yamlAlertConfig, e);
- responseMessage.put("message", "Failed to update the notification group " + id);
+ responseMessage.put("message", "Failed to update the notification group. Reach out to the ThirdEye team.");
responseMessage.put("more-info", "Error = " + e.getMessage());
return Response.serverError().entity(responseMessage).build();
}
LOG.info("Notification group " + id + " updated successfully with payload " + yamlAlertConfig);
responseMessage.put("message", "The YAML alert config was updated successfully.");
+ responseMessage.put("detectionAlertConfigId", String.valueOf(id));
return Response.ok().entity(responseMessage).build();
}
diff --git a/thirdeye/thirdeye-pinot/src/test/java/org/apache/pinot/thirdeye/detection/yaml/YamlResourceTest.java b/thirdeye/thirdeye-pinot/src/test/java/org/apache/pinot/thirdeye/detection/yaml/YamlResourceTest.java
index 7b53bfa..279340b 100644
--- a/thirdeye/thirdeye-pinot/src/test/java/org/apache/pinot/thirdeye/detection/yaml/YamlResourceTest.java
+++ b/thirdeye/thirdeye-pinot/src/test/java/org/apache/pinot/thirdeye/detection/yaml/YamlResourceTest.java
@@ -50,7 +50,7 @@ public class YamlResourceTest {
public void testCreateDetectionAlertConfig() throws IOException {
String blankYaml = "";
try {
- this.yamlResource.createDetectionAlertConfig(blankYaml);
+ this.yamlResource.createSubscriptionGroup(blankYaml);
Assert.fail("Exception not thrown on empty yaml");
} catch (Exception e) {
Assert.assertEquals(e.getMessage(), "The Yaml Payload in the request is empty.");
@@ -58,7 +58,7 @@ public class YamlResourceTest {
String inValidYaml = "application:test:application";
try {
- this.yamlResource.createDetectionAlertConfig(inValidYaml);
+ this.yamlResource.createSubscriptionGroup(inValidYaml);
Assert.fail("Exception not thrown on empty yaml");
} catch (Exception e) {
Assert.assertEquals(e.getMessage(), "Error parsing the Yaml input. Check for syntax issues.");
@@ -66,7 +66,7 @@ public class YamlResourceTest {
String noSubscriptGroupYaml = "application: test_application";
try {
- this.yamlResource.createDetectionAlertConfig(noSubscriptGroupYaml);
+ this.yamlResource.createSubscriptionGroup(noSubscriptGroupYaml);
Assert.fail("Exception not thrown on empty yaml");
} catch (Exception e) {
Assert.assertEquals(e.getMessage(), "Subscription group name field cannot be left empty.");
@@ -74,7 +74,7 @@ public class YamlResourceTest {
String appFieldMissingYaml = IOUtils.toString(this.getClass().getResourceAsStream("alertconfig/alert-config-1.yaml"));
try {
- this.yamlResource.createDetectionAlertConfig(appFieldMissingYaml);
+ this.yamlResource.createSubscriptionGroup(appFieldMissingYaml);
Assert.fail("Exception not thrown on empty yaml");
} catch (Exception e) {
Assert.assertEquals(e.getMessage(), "Application field cannot be left empty");
@@ -82,7 +82,7 @@ public class YamlResourceTest {
String appMissingYaml = IOUtils.toString(this.getClass().getResourceAsStream("alertconfig/alert-config-2.yaml"));
try {
- this.yamlResource.createDetectionAlertConfig(appMissingYaml);
+ this.yamlResource.createSubscriptionGroup(appMissingYaml);
Assert.fail("Exception not thrown on empty yaml");
} catch (Exception e) {
Assert.assertEquals(e.getMessage(), "Application name doesn't exist in our registry."
@@ -101,7 +101,7 @@ public class YamlResourceTest {
String groupExists = IOUtils.toString(this.getClass().getResourceAsStream("alertconfig/alert-config-3.yaml"));
try {
- this.yamlResource.createDetectionAlertConfig(groupExists);
+ this.yamlResource.createSubscriptionGroup(groupExists);
Assert.fail("Exception not thrown on empty yaml");
} catch (Exception e) {
Assert.assertEquals(e.getMessage(), "Subscription group name is already taken. Please use a different name.");
@@ -109,9 +109,10 @@ public class YamlResourceTest {
String validYaml = IOUtils.toString(this.getClass().getResourceAsStream("alertconfig/alert-config-4.yaml"));
try {
- DetectionAlertConfigDTO alert = this.yamlResource.createDetectionAlertConfig(validYaml);
- Assert.assertNotNull(alert);
- Assert.assertEquals(alert.getName(), "Subscription Group Name");
+ long id = this.yamlResource.createSubscriptionGroup(validYaml);
+ DetectionConfigDTO detection = daoRegistry.getDetectionConfigManager().findById(id);
+ Assert.assertNotNull(detection);
+ Assert.assertEquals(detection.getName(), "Subscription Group Name");
} catch (Exception e) {
Assert.fail("Exception should not be thrown for valid yaml");
}
@@ -127,7 +128,7 @@ public class YamlResourceTest {
DetectionAlertConfigDTO alertDTO;
try {
- this.yamlResource.updateDetectionAlertConfig(-1, "");
+ this.yamlResource.updateSubscriptionGroup(-1, "");
Assert.fail("Exception not thrown on empty yaml");
} catch (Exception e) {
Assert.assertEquals(e.getMessage(), "Cannot find subscription group -1");
@@ -135,7 +136,7 @@ public class YamlResourceTest {
String blankYaml = "";
try {
- this.yamlResource.updateDetectionAlertConfig(oldId, blankYaml);
+ this.yamlResource.updateSubscriptionGroup(oldId, blankYaml);
Assert.fail("Exception not thrown on empty yaml");
} catch (Exception e) {
Assert.assertEquals(e.getMessage(), "The Yaml Payload in the request is empty.");
@@ -143,7 +144,7 @@ public class YamlResourceTest {
String inValidYaml = "application:test:application";
try {
- this.yamlResource.updateDetectionAlertConfig(oldId, inValidYaml);
+ this.yamlResource.updateSubscriptionGroup(oldId, inValidYaml);
Assert.fail("Exception not thrown on empty yaml");
} catch (Exception e) {
Assert.assertEquals(e.getMessage(), "Error parsing the Yaml input. Check for syntax issues.");
@@ -151,7 +152,7 @@ public class YamlResourceTest {
String noSubscriptGroupYaml = "application: test_application";
try {
- this.yamlResource.updateDetectionAlertConfig(oldId, noSubscriptGroupYaml);
+ this.yamlResource.updateSubscriptionGroup(oldId, noSubscriptGroupYaml);
Assert.fail("Exception not thrown on empty yaml");
} catch (Exception e) {
Assert.assertEquals(e.getMessage(), "Subscription group name field cannot be left empty.");
@@ -159,7 +160,7 @@ public class YamlResourceTest {
String appFieldMissingYaml = IOUtils.toString(this.getClass().getResourceAsStream("alertconfig/alert-config-1.yaml"));
try {
- this.yamlResource.updateDetectionAlertConfig(oldId, appFieldMissingYaml);
+ this.yamlResource.updateSubscriptionGroup(oldId, appFieldMissingYaml);
Assert.fail("Exception not thrown on empty yaml");
} catch (Exception e) {
Assert.assertEquals(e.getMessage(), "Application field cannot be left empty");
@@ -172,7 +173,8 @@ public class YamlResourceTest {
String validYaml = IOUtils.toString(this.getClass().getResourceAsStream("alertconfig/alert-config-3.yaml"));
try {
- alertDTO = this.yamlResource.updateDetectionAlertConfig(oldId, validYaml);
+ this.yamlResource.updateSubscriptionGroup(oldId, validYaml);
+ alertDTO = daoRegistry.getDetectionAlertConfigManager().findById(oldId);
Assert.assertNotNull(alertDTO);
Assert.assertEquals(alertDTO.getName(), "test_group");
Assert.assertEquals(alertDTO.getApplication(), "test_application");
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org