samza-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Navina Ramesh" <nram...@linkedin.com>
Subject Re: Review Request 35445: SAMZA-693: Very basic HDFS Producer service for Samza
Date Wed, 29 Jul 2015 19:41:35 GMT

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


Just a few comments. Overall, looks good!


docs/learn/documentation/versioned/hdfs/producer.md (line 24)
<https://reviews.apache.org/r/35445/#comment147892>

    Can you please update the list of available configuration for this system at http://samza.apache.org/learn/documentation/latest/jobs/configuration-table.html
as well? 
    
    Like we did with elastic search, adding an example job to hello-samza will be very useful
for adoption. If not in this JIRA, please consider opening a follow-up JIRA to add this example.



samza-hdfs/src/main/scala/org/apache/samza/system/hdfs/writer/TextSequenceFileHdfsWriter.scala
(line 37)
<https://reviews.apache.org/r/35445/#comment147891>

    What is the motivation for having 2 sequenceFileHdfsWriters?  Is the only difference in
the data type being written to the fs? 
    
    The difference is not clear from the class's javadoc or the example configuration provided.
The reason I ask is we already have a config overload in Samza. I would rather not have the
user configure a writer type, if we can safely default to a single type and only override
the relevant diverging properties. Functionally, I believe they are doing the same thing.


- Navina Ramesh


On July 28, 2015, 5:25 a.m., Eli Reisman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35445/
> -----------------------------------------------------------
> 
> (Updated July 28, 2015, 5:25 a.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> SAMZA-693: Very basic HDFS Producer service for Samza
> 
> 
> Diffs
> -----
> 
>   build.gradle 0852adc 
>   docs/learn/documentation/versioned/hdfs/producer.md PRE-CREATION 
>   samza-hdfs/src/main/scala/org/apache/samza/system/hdfs/HdfsConfig.scala PRE-CREATION

>   samza-hdfs/src/main/scala/org/apache/samza/system/hdfs/HdfsSystemAdmin.scala PRE-CREATION

>   samza-hdfs/src/main/scala/org/apache/samza/system/hdfs/HdfsSystemFactory.scala PRE-CREATION

>   samza-hdfs/src/main/scala/org/apache/samza/system/hdfs/HdfsSystemProducer.scala PRE-CREATION

>   samza-hdfs/src/main/scala/org/apache/samza/system/hdfs/HdfsSystemProducerMetrics.scala
PRE-CREATION 
>   samza-hdfs/src/main/scala/org/apache/samza/system/hdfs/writer/BinarySequenceFileHdfsWriter.scala
PRE-CREATION 
>   samza-hdfs/src/main/scala/org/apache/samza/system/hdfs/writer/Bucketer.scala PRE-CREATION

>   samza-hdfs/src/main/scala/org/apache/samza/system/hdfs/writer/HdfsWriter.scala PRE-CREATION

>   samza-hdfs/src/main/scala/org/apache/samza/system/hdfs/writer/JobNameDateTimeBucketer.scala
PRE-CREATION 
>   samza-hdfs/src/main/scala/org/apache/samza/system/hdfs/writer/SequenceFileHdfsWriter.scala
PRE-CREATION 
>   samza-hdfs/src/main/scala/org/apache/samza/system/hdfs/writer/TextSequenceFileHdfsWriter.scala
PRE-CREATION 
>   samza-hdfs/src/test/resources/samza-hdfs-test-batch-job-text.properties PRE-CREATION

>   samza-hdfs/src/test/resources/samza-hdfs-test-batch-job.properties PRE-CREATION 
>   samza-hdfs/src/test/resources/samza-hdfs-test-job-text.properties PRE-CREATION 
>   samza-hdfs/src/test/resources/samza-hdfs-test-job.properties PRE-CREATION 
>   samza-hdfs/src/test/scala/org/apache/samza/system/hdfs/TestHdfsSystemProducerTestSuite.scala
PRE-CREATION 
>   settings.gradle 19bff97 
> 
> Diff: https://reviews.apache.org/r/35445/diff/
> 
> 
> Testing
> -------
> 
> Updated: See JIRA SAMZA-693 for details, this latest update (693-4) addresses post-review
issues and adds more pluggable design, several default writer implementations, and more (and
more thorough) unit tests.
> 
> Passes 'gradle clean test'.
> 
> 
> Thanks,
> 
> Eli Reisman
> 
>


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