You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2022/03/02 14:46:26 UTC

[GitHub] [nifi] ChrisSamo632 commented on a change in pull request #5822: NIFI-9678 update Elasticsearch REST API processor integration-tests for Elasticsearch 8.x

ChrisSamo632 commented on a change in pull request #5822:
URL: https://github.com/apache/nifi/pull/5822#discussion_r817761914



##########
File path: nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-client-service/src/test/groovy/org/apache/nifi/elasticsearch/integration/ElasticSearchClientService_IT.groovy
##########
@@ -223,9 +226,13 @@ class ElasticSearchClientService_IT {
 
     @Test
     void testSearchWarnings() {
+        Assume.assumeTrue("Requires version <8.0 (no search API deprecations yet for 8.x)", VERSION < ES_8_0)
+
         String query
         String type = TYPE
-        if (VERSION.toString().startsWith("7.")) {
+        if (VERSION.toString().startsWith("8.")) {
+            // TODO: something that's deprecated when the 8.x branch progresses to include search-API deprecations

Review comment:
       I'm generally not a fan of TODOs in code, but there's literally no deprecated Search API funcitons in Elasticsearch 8.x yet (it's too new for them to have deprecated anything, having just removed all previously deprecated stuff from 7.x).
   
   I've Skipped the test for 8.x using the `Assume` but I think we should look to update this test to prove the WARNING funcitnoality for 8.x once Search API deprecations appear in Elasticsearch (so that would be a case of removing the `Assume` and implementing some sort of query here that caused a WARNING).
   
   We could just remove this TODO and `if` branch, but I thought the TODO here would hopefully serve as a reminder to do this in future. I'd normally raise a Jira ticket to cover a TODO, but as there's no realistic way of currently implementing such a Jira ticket, it seemed like that wouldn't be sensible either.
   
   So I think the options are:
   1. Leave as-is and pick this up for a future version of Elasticsearch 8.x
   2. Remove the TODO and hope someone picks this up in future
   3. Raise a Jira ticket to pick this up in future, noting that it can't currently be implemented and someone will need to watch out for an appropriate Search API deprecation in order to achieve the test




-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org