gresockj commented on a change in pull request #5665:
URL: https://github.com/apache/nifi/pull/5665#discussion_r809891315
##########
File path:
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/PutElasticsearchJson.java
##########
@@ -95,27 +101,36 @@
.name("put-es-json-error-documents")
.displayName("Output Error Documents")
.description("If this configuration property is true, the response
from Elasticsearch will be examined for failed documents " +
- "and the failed documents will be sent to the \"errors\"
relationship.")
+ "and the failed documents will be sent to the \"" +
REL_FAILED_DOCUMENTS.getName() + "\" relationship.")
.allowableValues("true", "false")
.defaultValue("false")
.expressionLanguageSupported(ExpressionLanguageScope.NONE)
.required(true)
.build();
- static final Relationship REL_FAILED_DOCUMENTS = new Relationship.Builder()
- .name("errors").description("If \"" +
OUTPUT_ERROR_DOCUMENTS.getDisplayName() + "\" is set, " +
- "any FlowFile that failed to process the way it was
configured will be sent to this relationship " +
- "as part of a failed document set.")
- .autoTerminateDefault(true).build();
+ static final PropertyDescriptor NOT_FOUND_IS_SUCCESSFUL = new
PropertyDescriptor.Builder()
+ .name("put-es-not_found-is-error")
+ .displayName("Treat \"Not Found\" as Error")
+ .description("If \"" + OUTPUT_ERROR_DOCUMENTS.getName() + "\" is true,
\"not_found\" Elasticsearch Documents " +
Review comment:
Since this depends on OUTPUT_ERROR_DOCUMENTS, I don't think you need to
add the conditional language here, since any time this property is exposed, you
have OUTPUT_ERROR_DOCUMENTS set to true. Instead, I think this could be more
clear by saying: "If true, 'not_found' Elasticsearch Documents will be routed
to the 'success' relationship; otherwise they are routed to 'error'."
##########
File path:
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/PutElasticsearchRecord.java
##########
@@ -86,6 +96,16 @@
resource = SystemResource.MEMORY,
description = "The Batch of Records will be stored in memory until the
bulk operation is performed.")
public class PutElasticsearchRecord extends AbstractPutElasticsearch {
+ static final Relationship REL_FAILED_RECORDS = new Relationship.Builder()
+ .name("errors").description("If a \"Result Record Writer\" is set,
any record that failed to process the way it was " +
+ "configured will be sent to this relationship as part of a
failed record set.")
+ .autoTerminateDefault(true).build();
+
+ static final Relationship REL_SUCCESSFUL_RECORDS = new
Relationship.Builder()
+ .name("successful").description("If a \"Result Record Writer\" is
set, any record that successfully processed the way it was " +
Review comment:
Could use the same language as suggested above
##########
File path:
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/PutElasticsearchRecord.java
##########
@@ -86,6 +96,16 @@
resource = SystemResource.MEMORY,
description = "The Batch of Records will be stored in memory until the
bulk operation is performed.")
public class PutElasticsearchRecord extends AbstractPutElasticsearch {
+ static final Relationship REL_FAILED_RECORDS = new Relationship.Builder()
+ .name("errors").description("If a \"Result Record Writer\" is set,
any record that failed to process the way it was " +
Review comment:
I find this "the way it was configured" language confusing. Since I
think this relationship is about capturing Record Writer errors, we might take
the opportunity to update this to:
"If a 'Result Record Writer' is set, any record that fails during Record
Writing will be sent to this relationship as part of a failed record set."
##########
File path:
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/PutElasticsearchRecord.java
##########
@@ -182,17 +202,31 @@
.expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
.build();
- static final PropertyDescriptor ERROR_RECORD_WRITER = new
PropertyDescriptor.Builder()
+ static final PropertyDescriptor RESULT_RECORD_WRITER = new
PropertyDescriptor.Builder()
.name("put-es-record-error-writer")
- .displayName("Error Record Writer")
+ .displayName("Result Record Writer")
.description("If this configuration property is set, the response from
Elasticsearch will be examined for failed records " +
- "and the failed records will be written to a record set with
this record writer service and sent to the \"errors\" " +
- "relationship.")
+ "and the failed records will be written to a record set with
this record writer service and sent to the \"" +
+ REL_FAILED_RECORDS.getName() + "\" relationship. Successful
records will be written to a record set" +
+ "with this record writer service and sent to the \"" +
REL_SUCCESSFUL_RECORDS.getName() + "\" relationship.")
.identifiesControllerService(RecordSetWriterFactory.class)
.addValidator(Validator.VALID)
.required(false)
.build();
+ static final PropertyDescriptor NOT_FOUND_IS_SUCCESSFUL = new
PropertyDescriptor.Builder()
+ .name("put-es-not_found-is-error")
+ .displayName("Treat \"Not Found\" as Error")
+ .description("If a \"" + RESULT_RECORD_WRITER.getName() + "\" is set,
\"not_found\" Elasticsearch Documents " +
Review comment:
Similar suggestion as in `PutElasticsearchJson`
##########
File path:
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/PutElasticsearchRecord.java
##########
@@ -86,6 +96,16 @@
resource = SystemResource.MEMORY,
description = "The Batch of Records will be stored in memory until the
bulk operation is performed.")
public class PutElasticsearchRecord extends AbstractPutElasticsearch {
+ static final Relationship REL_FAILED_RECORDS = new Relationship.Builder()
+ .name("errors").description("If a \"Result Record Writer\" is set,
any record that failed to process the way it was " +
+ "configured will be sent to this relationship as part of a
failed record set.")
+ .autoTerminateDefault(true).build();
+
+ static final Relationship REL_SUCCESSFUL_RECORDS = new
Relationship.Builder()
Review comment:
I understand the naming challenge here, since it was perhaps more
descriptive to call the failed records relationship something like "record
errors" and we don't want to break backwards compatibility, but I think the
"success" vs "successful" is potentially too confusing. I'd recommend calling
this "successful records".
##########
File path:
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-restapi-processors/src/main/java/org/apache/nifi/processors/elasticsearch/PutElasticsearchRecord.java
##########
@@ -230,24 +264,21 @@
.required(false)
.build();
- static final Relationship REL_FAILED_RECORDS = new Relationship.Builder()
- .name("errors").description("If an Error Record Writer is set, any
record that failed to process the way it was " +
- "configured will be sent to this relationship as part of a
failed record set.")
- .autoTerminateDefault(true).build();
-
static final List<PropertyDescriptor> DESCRIPTORS =
Collections.unmodifiableList(Arrays.asList(
INDEX_OP, INDEX, TYPE, AT_TIMESTAMP, CLIENT_SERVICE, RECORD_READER,
BATCH_SIZE, ID_RECORD_PATH, RETAIN_ID_FIELD,
INDEX_OP_RECORD_PATH, INDEX_RECORD_PATH, TYPE_RECORD_PATH,
AT_TIMESTAMP_RECORD_PATH, RETAIN_AT_TIMESTAMP_FIELD,
- DATE_FORMAT, TIME_FORMAT, TIMESTAMP_FORMAT, LOG_ERROR_RESPONSES,
ERROR_RECORD_WRITER
+ DATE_FORMAT, TIME_FORMAT, TIMESTAMP_FORMAT, LOG_ERROR_RESPONSES,
RESULT_RECORD_WRITER, NOT_FOUND_IS_SUCCESSFUL
));
static final Set<Relationship> RELATIONSHIPS =
Collections.unmodifiableSet(new HashSet<>(Arrays.asList(
- REL_SUCCESS, REL_FAILURE, REL_RETRY, REL_FAILED_RECORDS
+ REL_SUCCESS, REL_FAILURE, REL_RETRY, REL_FAILED_RECORDS,
REL_SUCCESSFUL_RECORDS
)));
private RecordPathCache recordPathCache;
private RecordReaderFactory readerFactory;
private RecordSetWriterFactory writerFactory;
+ private boolean notFoundIsSuccessful;
Review comment:
I know this won't have a huge impact, but is there an opportunity for
reuse of this variable since it's used in both concrete implementations of
`AbstractPutElasticsearch`?
--
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-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
