Thank you, Dan. I think we're ready to merge. Can one of the Samza committers please take a look?
On Wed, Jul 29, 2015 at 11:31 AM, Dan Harvey <danharve...@gmail.com> wrote: > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36815/ > > On July 29th, 2015, 8:42 a.m. UTC, *Dan Harvey* wrote: > > > samza-elasticsearch/src/main/java/org/apache/samza/system/elasticsearch/ElasticsearchSystemProducer.java > <https://reviews.apache.org/r/36815/diff/6/?file=1024157#file1024157line116> > (Diff > revision 6) > > public void register(final String source) { > > 116 > > LOGGER.info(itemResp.getFailureMessage()); > > Should we add a Samza specifc message, then add the whole exception? so > it's more clear what the exception was from if the user doesn't know the > code? Logger.info("Failed to index message in ElasticSearch.", e); > > This would also be true for other log lines added. > > On July 29th, 2015, 6:22 p.m. UTC, *Roger Hoover* wrote: > > Good idea. Thanks. > > BTW, it didn't work like this: Logger.info("Failed to index message in > ElasticSearch.", itemResp.getFailure()) so I did this: > > LOGGER.error("Failed to index document in Elasticsearch: " + > itemResp.getFailureMessage()); > > On July 29th, 2015, 6:28 p.m. UTC, *Roger Hoover* wrote: > > This is what the messages look like > > 2015-07-29 11:15:02 ElasticsearchSystemProducer [INFO] Failed to index > document in Elasticsearch: > VersionConflictEngineException[[test-embedded.2015-07-29][0] [stuff][d3]: > version conflict, current [9], provided [5]] > > That looks fine! > > > - Dan > > On July 29th, 2015, 6:24 p.m. UTC, Roger Hoover wrote: > Review request for samza and Dan Harvey. > By Roger Hoover. > > *Updated July 29, 2015, 6:24 p.m.* > *Repository: * samza > Description > > SAMZA-741 Add support for versioning to Elasticsearch System Producer > > Testing > > Refactored DefaultIndexRequestFactory to make it easier to subclass and > customize to handle version and version_type parameters. > > Diffs > > - > samza-elasticsearch/src/main/java/org/apache/samza/system/elasticsearch/ElasticsearchSystemProducer.java > (f61bd36) > - > samza-elasticsearch/src/main/java/org/apache/samza/system/elasticsearch/ElasticsearchSystemProducerMetrics.java > (e3b635b) > - > samza-elasticsearch/src/main/java/org/apache/samza/system/elasticsearch/indexrequest/DefaultIndexRequestFactory.java > (afe0eee) > - > samza-elasticsearch/src/test/java/org/apache/samza/system/elasticsearch/ElasticsearchSystemProducerMetricsTest.java > (980964f) > - > samza-elasticsearch/src/test/java/org/apache/samza/system/elasticsearch/ElasticsearchSystemProducerTest.java > (684d7f6) > > View Diff <https://reviews.apache.org/r/36815/diff/> >
