Github user merrimanr commented on a diff in the pull request:
https://github.com/apache/metron/pull/1239#discussion_r226460423
--- Diff:
metron-platform/metron-writer/src/main/java/org/apache/metron/writer/BulkWriterComponent.java
---
@@ -118,12 +118,15 @@ public void commit(BulkWriterResponse response) {
public void error(String sensorType, Throwable e, Iterable<Tuple>
tuples, MessageGetStrategy messageGetStrategy) {
LOG.error(format("Failing %d tuple(s); sensorType=%s",
Iterables.size(tuples), sensorType), e);
- MetronError error = new MetronError()
- .withSensorType(Collections.singleton(sensorType))
- .withErrorType(Constants.ErrorType.INDEXING_ERROR)
- .withThrowable(e);
- tuples.forEach(t -> error.addRawMessage(messageGetStrategy.get(t)));
- handleError(tuples, error);
+ tuples.forEach(t -> {
--- End diff --
You're right again. I agree, we shouldn't be reporting the same exception
for every message in the batch. I changed it to what you suggested in the
initial comment and updated the tests.
For the other issue you bring up with `error(Throwable, Iterable<Tuple>)`,
it looks like it gets called from only one place:
`BulkMessageWriterBolt.handleMissingMessage`. This method only deals with a
single tuple so it is misleading that the `error` method accepts multiple
tuples. Would changing that to accept a single tuple make it less confusing?
---
