Code review fixes

Signed-off-by: Daniel Widdis <[email protected]>
opensearch-project · Feb 7, 2023 · 7f68dc7 · 7f68dc7
1 parent 01dd362
commit 7f68dc7
Show file tree

Hide file tree

Showing 6 changed files with 18 additions and 25 deletions.
diff --git a/src/main/java/org/opensearch/ad/feature/SearchFeatureDao.java b/src/main/java/org/opensearch/ad/feature/SearchFeatureDao.java
@@ -123,7 +123,8 @@ public SearchFeatureDao(
             settingsUpdateConsumers.put(PAGE_SIZE, it -> this.pageSize = (int) it);
             clusterService.getClusterSettings().addSettingsUpdateConsumer(settingsUpdateConsumers);
         } catch (Exception e) {
-            // TODO Handle this
+            // FIXME Handle this
+            // https://github.com/opensearch-project/opensearch-sdk-java/issues/422
         }
         this.minimumDocCountForPreview = minimumDocCount;
         this.previewTimeoutInMilliseconds = previewTimeoutInMilliseconds;

diff --git a/src/main/java/org/opensearch/ad/indices/AnomalyDetectionIndices.java b/src/main/java/org/opensearch/ad/indices/AnomalyDetectionIndices.java
@@ -178,6 +178,8 @@ public AnomalyDetectionIndices(
         this.adminClient = restClient;
         this.clusterService = sdkClusterService;
         this.threadPool = threadPool;
+        // FIXME Implement this
+        // https://github.com/opensearch-project/opensearch-sdk-java/issues/423
         // this.clusterService.addLocalNodeMasterListener(this);
         this.historyRolloverPeriod = AD_RESULT_HISTORY_ROLLOVER_PERIOD.get(settings);
         this.historyMaxDocs = AD_RESULT_HISTORY_MAX_DOCS_PER_SHARD.get(settings);
@@ -203,7 +205,8 @@ public AnomalyDetectionIndices(
         try {
             this.clusterService.getClusterSettings().addSettingsUpdateConsumer(settingToConsumerMap);
         } catch (Exception e) {
-            // TODO Handle this
+            // FIXME Handle this
+            // https://github.com/opensearch-project/opensearch-sdk-java/issues/422
         }
 
         this.settings = Settings.builder().put("index.hidden", true).build();

diff --git a/src/main/java/org/opensearch/ad/rest/RestValidateAnomalyDetectorAction.java b/src/main/java/org/opensearch/ad/rest/RestValidateAnomalyDetectorAction.java
@@ -49,8 +49,6 @@
 import org.opensearch.common.xcontent.json.JsonXContent;
 import org.opensearch.extensions.rest.ExtensionRestRequest;
 import org.opensearch.extensions.rest.ExtensionRestResponse;
-import org.opensearch.rest.BytesRestResponse;
-import org.opensearch.rest.RestChannel;
 import org.opensearch.rest.RestRequest;
 import org.opensearch.rest.RestStatus;
 import org.opensearch.sdk.ExtensionsRunner;
@@ -118,16 +116,13 @@ public List<RouteHandler> routeHandlers() {
         }
     };
 
-    protected void sendAnomalyDetectorValidationParseResponse(DetectorValidationIssue issue, RestChannel channel) throws IOException {
-        try {
-            BytesRestResponse restResponse = new BytesRestResponse(
-                RestStatus.OK,
-                new ValidateAnomalyDetectorResponse(issue).toXContent(channel.newBuilder())
-            );
-            channel.sendResponse(restResponse);
-        } catch (Exception e) {
-            channel.sendResponse(new BytesRestResponse(RestStatus.INTERNAL_SERVER_ERROR, e.getMessage()));
-        }
+    protected ExtensionRestResponse sendAnomalyDetectorValidationParseResponse(ExtensionRestRequest request, DetectorValidationIssue issue)
+        throws IOException {
+        return new ExtensionRestResponse(
+            request,
+            RestStatus.OK,
+            new ValidateAnomalyDetectorResponse(issue).toXContent(JsonXContent.contentBuilder())
+        );
     }
 
     private Boolean validationTypesAreAccepted(String validationType) {
@@ -161,11 +156,7 @@ protected ExtensionRestResponse prepareRequest(ExtensionRestRequest request) thr
                     ADException.getType(),
                     ADException.getMessage()
                 );
-                return new ExtensionRestResponse(
-                    request,
-                    RestStatus.OK,
-                    new ValidateAnomalyDetectorResponse(issue).toXContent(JsonXContent.contentBuilder())
-                );
+                return sendAnomalyDetectorValidationParseResponse(request, issue);
             } else {
                 throw ex;
             }

diff --git a/src/main/java/org/opensearch/ad/transport/IndexAnomalyDetectorTransportAction.java b/src/main/java/org/opensearch/ad/transport/IndexAnomalyDetectorTransportAction.java
@@ -84,7 +84,8 @@ public IndexAnomalyDetectorTransportAction(
         try {
             sdkClusterService.getClusterSettings().addSettingsUpdateConsumer(FILTER_BY_BACKEND_ROLES, it -> filterByEnabled = it);
         } catch (Exception e) {
-            // TODO Handle this
+            // FIXME Handle this
+            // https://github.com/opensearch-project/opensearch-sdk-java/issues/422
         }
     }
 

diff --git a/src/main/java/org/opensearch/ad/transport/ValidateAnomalyDetectorTransportAction.java b/src/main/java/org/opensearch/ad/transport/ValidateAnomalyDetectorTransportAction.java
@@ -83,7 +83,8 @@ public ValidateAnomalyDetectorTransportAction(
         try {
             clusterService.getClusterSettings().addSettingsUpdateConsumer(FILTER_BY_BACKEND_ROLES, it -> filterByEnabled = it);
         } catch (Exception e) {
-            // TODO Handle this
+            // FIXME Handle this
+            // https://github.com/opensearch-project/opensearch-sdk-java/issues/422
         }
         this.searchFeatureDao = searchFeatureDao;
         this.clock = Clock.systemUTC();

diff --git a/src/main/java/org/opensearch/ad/util/ParseUtils.java b/src/main/java/org/opensearch/ad/util/ParseUtils.java
@@ -523,11 +523,8 @@ public static void getDetector(
         NamedXContentRegistry xContentRegistry,
         boolean filterByBackendRole
     ) {
-        logger.info("in getDetector()");
         if (clusterService.state().metadata().indices().containsKey(AnomalyDetector.ANOMALY_DETECTORS_INDEX)) {
-            logger.info("Cluster metadata contains {}", AnomalyDetector.ANOMALY_DETECTORS_INDEX);
             GetRequest request = new GetRequest(AnomalyDetector.ANOMALY_DETECTORS_INDEX).id(detectorId);
-            logger.info("Creating get request for detector {}", detectorId);
             client
                 .get(
                     request,
@@ -549,7 +546,6 @@ public static void getDetector(
                         )
                 );
         } else {
-            logger.info("Index not found: {}", AnomalyDetector.ANOMALY_DETECTORS_INDEX);
             listener.onFailure(new IndexNotFoundException(AnomalyDetector.ANOMALY_DETECTORS_INDEX));
         }
     }