akshayrai commented on a change in pull request #5011: [TE] endpoint -
harleyjj/yamlresource - add dataset as an optional pa…
URL: https://github.com/apache/incubator-pinot/pull/5011#discussion_r371965339
##########
File path:
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/YamlResource.java
##########
@@ -904,15 +904,64 @@ public Response toggleActivation(
return Response.ok(responseMessage).build();
}
+ /**
+ * Implements filter for detection config.
+ * Client can filter by dataset or metric or both. It only filters if they
are not null in params and config
+ *
+ * @param detectionConfig The detection configuration after being enhanced
by DetectionConfigFormatter::format.
+ * @param dataset The dataset param passed by the client in the REST API
call.
+ * @param metric The metric param passed by the client in the REST API call.
+ * @return true if the config matches query params that are passed by client.
+ */
+ private boolean filterConfigsBy(Map<String, Object> detectionConfig, String
dataset, String metric) {
+ List datasetList = (List) detectionConfig.get("datasetNames");
+ String metricString = (String) detectionConfig.get("metric");
+ // defaults are true so we filter only if the params are passed
+ boolean metricMatch = true;
+ boolean datasetMatch = true;
+ // check metric only if it was passed
+ if (metric != null ) {
+ // equals method should not be called on null
+ metricMatch = metricString != null && metricString.equals(metric);
+ }
+ // check dataset only if it was passed
+ if (dataset != null) {
+ // contains method should not be called on null
+ datasetMatch = datasetList != null && datasetList.contains(dataset);
+ }
+ // config should satisfy both filters
+ return metricMatch && datasetMatch;
+ }
+
/**
* List all yaml configurations as JSON enhanced with detection config id,
isActive and createBy information.
+ * @param dataset the dataset to filter results by (optional)
+ * @param metric the metric to filter results by (optional)
* @return the yaml configuration converted in to JSON, with enhanced
information from detection config DTO.
*/
@GET
@Path("/list")
+ @ApiOperation("Get the list of all detection YAML configurations as JSON
enhanced with additional information, optionally filtered.")
@Produces(MediaType.APPLICATION_JSON)
- public List<Map<String, Object>> listYamls() throws ExecutionException {
- return
this.detectionConfigDAO.findAll().parallelStream().map(this.detectionConfigFormatter::format).collect(Collectors.toList());
+ public List<Map<String, Object>> listYamls(
+ @ApiParam("Dataset the detection configurations should be filtered by")
@QueryParam("dataset") String dataset,
+ @ApiParam("Metric the detection configurations should be filtered by")
@QueryParam("metric") String metric) throws ExecutionException {
+ List<Map<String, Object>> yamls;
+ if (dataset == null && metric == null) {
+ yamls = this.detectionConfigDAO
+ .findAll()
+ .parallelStream()
+ .map(this.detectionConfigFormatter::format)
+ .collect(Collectors.toList());
+ } else {
+ yamls = this.detectionConfigDAO
+ .findAll()
+ .parallelStream()
+ .map(this.detectionConfigFormatter::format)
+ .filter(y -> filterConfigsBy(y, dataset, metric))
+ .collect(Collectors.toList());
+ }
+ return yamls;
}
Review comment:
Although the legacy code doesn't follow the standards, I have some
suggestions which we have been trying to follow for all the new apis.
1. If there is an exception thrown during the execution then we need to wrap
it up and return a meaningful response to the caller along with the status
codes.
2. Also consider adding some logging statements indicating how many
configurations were returned for the params passed.
Refer example:
https://github.com/apache/incubator-pinot/blob/master/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/api/user/dashboard/UserDashboardResource.java#L220
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
