[GitHub] [nifi] ChrisSamo632 commented on a diff in pull request #6903: NIFI-11111 add option to output Elasticsearch error responses as FlowFile to PutElasticsearchJson and PutElasticsearchRecord

["ChrisSamo632 (via GitHub)" <gi...@apache.org>]

ChrisSamo632 commented on code in PR #6903:
URL: https://github.com/apache/nifi/pull/6903#discussion_r1103219574


##########
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/AbstractPutElasticsearch.java:
##########
@@ -67,13 +69,28 @@ public abstract class AbstractPutElasticsearch extends AbstractProcessor impleme
         .required(true)
         .build();
 
+    static final PropertyDescriptor OUTPUT_ERROR_RESPONSES = new PropertyDescriptor.Builder()
+            .name("put-es-output-error-responses")
+            .displayName("Output Error Responses")
+            .description("If this is enabled, errors will be output to the \"error_responses\" relationship.")
+            .allowableValues("true", "false")
+            .defaultValue("false")
+            .addValidator(StandardValidators.BOOLEAN_VALIDATOR)
+            .build();
+
     static final Relationship REL_SUCCESS = new Relationship.Builder()
             .name("success")
             .description("All flowfiles that succeed in being transferred into Elasticsearch go here. " +
                     "Documents received by the Elasticsearch _bulk API may still result in errors on the Elasticsearch side. " +
                     "The Elasticsearch response will need to be examined to determine whether any Document(s)/Record(s) resulted in errors.")
             .build();
 
+    static final Relationship REL_ERROR_RESPONSES = new Relationship.Builder()
+            .name("error_responses")
+            .description("Elasticsearch _bulk API responses marked as \"error\" (and optionally \"not_found\") go here.")

Review Comment:
   This depends on the `NOT_FOUND_IS_SUCCESSFUL` PropertyDescriptors in the two implementing classes (existing properties).
   
   I've update this relationship's description along with the existing `PropertyDescriptor`s to reflect this (also corrected the `displayName`s of those properties as they were misleading).



##########
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/AbstractPutElasticsearch.java:
##########
@@ -67,13 +69,28 @@ public abstract class AbstractPutElasticsearch extends AbstractProcessor impleme
         .required(true)
         .build();
 
+    static final PropertyDescriptor OUTPUT_ERROR_RESPONSES = new PropertyDescriptor.Builder()
+            .name("put-es-output-error-responses")
+            .displayName("Output Error Responses")
+            .description("If this is enabled, errors will be output to the \"error_responses\" relationship.")

Review Comment:
   I've updated the property description to indicate that the (optional) output of responses messages from Elasticsearch is indepenedant of the output of Records (or FlowFiles) from these processors
   
   I've changed the relationship from being autoTerminated and instead it will dynamically be added (and therefore need to be connected downstream) if this property is set to `true`



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