METRON-197: Validation should be the last step in the ParserBolt #143
Conversation
|
+1. This is a simple code change in the order of method calls. Builds and all tests pass |
| if(!isGloballyValid(message, fieldValidations)) { | ||
| message.put(Constants.SENSOR_TYPE, getSensorType()+ ".invalid"); | ||
| collector.emit(Constants.INVALID_STREAM, new Values(message)); | ||
| } | ||
| else if (filter != null && filter.emitTuple(message)) { | ||
| else { | ||
| ackTuple = !isBulk; |
There was a problem hiding this comment.
Shouldn't the tuple get acked regardless of whether it passed the validations or not?
There was a problem hiding this comment.
The tuple will be acked. Note that ackTuple is initialized to true and is only set to false if we are in a situation where we are doing a bulk write, in which case the tuple will be acked in the bulk writer upon the writing of a batch of entries. So, when this loop is ended, it will ack on line 164.
|
I was under the impression that validations should happen after the field transformations. Is that not the case? |
|
If a message or a tuple fails validation then the associated message should get routed to a dead letter queue. I believe there is a different PR that is required to get the dead letter queue established. The tuple should be acked, though. We need to differentiate between if the tuple is failed due to not being able to be parsed vs. not validated. So if we fail based on parser issues then we need to ack for validation issues and then land the message in the dead letter queue. The important thing is for the messages that fail validation not to go through the rest of the pipeline. |
|
@merrimanr @james-sirota I tend to agree, validations should happen prior to field transformations since the transformation may affect the validity of the message. I'll adjust that. Good feedback! |
|
+1 |
Right now we are doing the validation prior to the messageFilter. We should only validate the parsed messages which passes through the filter.