samza-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jakob Homan" <jgho...@apache.org>
Subject Re: Review Request 33297: [SAMZA-654] Added ElasticsearchSystemProducer and Factory to output messages into Elasticseach indexes.
Date Fri, 17 Apr 2015 21:49:26 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33297/#review80538
-----------------------------------------------------------



samza-elasticsearch/src/main/java/org/apache/samza/config/ElasticsearchConfig.java
<https://reviews.apache.org/r/33297/#comment130537>

    need ASF Header



samza-elasticsearch/src/main/java/org/apache/samza/config/ElasticsearchConfig.java
<https://reviews.apache.org/r/33297/#comment130538>

    Should this implement Samza's config interface?



samza-elasticsearch/src/main/java/org/apache/samza/system/elasticsearch/ElasticsearchSystemAdmin.java
<https://reviews.apache.org/r/33297/#comment130539>

    header



samza-elasticsearch/src/main/java/org/apache/samza/system/elasticsearch/ElasticsearchSystemAdmin.java
<https://reviews.apache.org/r/33297/#comment130540>

    This is interesting... I don't imagine elasticsearch will ever be an appropriate store
for a changelog.  Should we throw an exception here?



samza-elasticsearch/src/main/java/org/apache/samza/system/elasticsearch/ElasticsearchSystemFactory.java
<https://reviews.apache.org/r/33297/#comment130541>

    header



samza-elasticsearch/src/main/java/org/apache/samza/system/elasticsearch/ElasticsearchSystemFactory.java
<https://reviews.apache.org/r/33297/#comment130544>

    Since the admin is doing nothing, should we just have a single instance of it to return?



samza-elasticsearch/src/main/java/org/apache/samza/system/elasticsearch/ElasticsearchSystemProducer.java
<https://reviews.apache.org/r/33297/#comment130542>

    footer.
    
    just kidding. header.



samza-elasticsearch/src/main/java/org/apache/samza/system/elasticsearch/ElasticsearchSystemProducer.java
<https://reviews.apache.org/r/33297/#comment130545>

    Can we catch for invalid casts here and throw with a more verbose exception, to aid in
future debugging?



settings.gradle
<https://reviews.apache.org/r/33297/#comment130536>

    This list of non-Scala projects to check is getting too long and difficult to read.  Can
we just the relevant projects into a 'non-scala' map and check for inclusion?


- Jakob Homan


On April 17, 2015, 3:47 a.m., Dan Harvey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33297/
> -----------------------------------------------------------
> 
> (Updated April 17, 2015, 3:47 a.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> [SAMZA-654] Added ElasticsearchSystemProducer and Factory to output messages into Elasticseach
indexes.
> 
> 
> Diffs
> -----
> 
>   build.gradle 97de3a28f6379e3862eec845da87587b1d4f742e 
>   gradle/dependency-versions.gradle ee6dfc411b7ab90b187df79f109884127953862e 
>   samza-elasticsearch/src/main/java/org/apache/samza/config/ElasticsearchConfig.java
PRE-CREATION 
>   samza-elasticsearch/src/main/java/org/apache/samza/system/elasticsearch/ElasticsearchSystemAdmin.java
PRE-CREATION 
>   samza-elasticsearch/src/main/java/org/apache/samza/system/elasticsearch/ElasticsearchSystemFactory.java
PRE-CREATION 
>   samza-elasticsearch/src/main/java/org/apache/samza/system/elasticsearch/ElasticsearchSystemProducer.java
PRE-CREATION 
>   settings.gradle bb07a3b84b14dcef94da1bb166eab6aa3d0026bb 
> 
> Diff: https://reviews.apache.org/r/33297/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Harvey
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message