Thanks, Yi! On Wed, Jul 29, 2015 at 12:16 PM, Yi Pan <nickpa...@gmail.com> wrote:
> Hi, Roger, > > I am testing the patch now. Will update the JIRA soon. > > Thanks! > > -Yi > > On Wed, Jul 29, 2015 at 12:11 PM, Roger Hoover <roger.hoo...@gmail.com> > wrote: > > > 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/> > > > > > >
