> On June 25, 2015, 6:35 p.m., Yan Fang wrote: > > samza-hdfs/src/main/scala/org/apache/samza/system/hdfs/HdfsSystemProducer.scala, > > line 62 > > <https://reviews.apache.org/r/35445/diff/1/?file=984548#file984548line62> > > > > I would reommend all the log msgs follow the same format with other > > Samza code by removing the "s".
Excellent, will do > On June 25, 2015, 6:35 p.m., Yan Fang wrote: > > samza-hdfs/src/main/scala/org/apache/samza/system/hdfs/HdfsSystemProducer.scala, > > line 76 > > <https://reviews.apache.org/r/35445/diff/1/?file=984548#file984548line76> > > > > We can have the debug information there. Sounds good, I can uncomment that. > On June 25, 2015, 6:35 p.m., Yan Fang wrote: > > samza-hdfs/src/test/resources/samza-hdfs-test-job.properties, line 1 > > <https://reviews.apache.org/r/35445/diff/1/?file=984551#file984551line1> > > > > Is this used anywhere? We can put it in the testing class since it only > > has one property. I'll take a look. It might be this allowed me to configure the test producer in the same way as a real producer usage. If I'm remembering wrong I can move it as you say. > On June 25, 2015, 6:35 p.m., Yan Fang wrote: > > samza-hdfs/src/main/scala/org/apache/samza/system/hdfs/HdfsSystemProducer.scala, > > line 47 > > <https://reviews.apache.org/r/35445/diff/1/?file=984548#file984548line47> > > > > make this configurable as well? That sounds good > On June 25, 2015, 6:35 p.m., Yan Fang wrote: > > samza-hdfs/src/main/scala/org/apache/samza/system/hdfs/HdfsSystemProducer.scala, > > line 58 > > <https://reviews.apache.org/r/35445/diff/1/?file=984548#file984548line58> > > > > aggree to get it pluggable. Agreed, I have an idea how we might do that that I'll post in the next patch. Thanks again for the review! - Eli ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35445/#review89390 ----------------------------------------------------------- On June 14, 2015, 10:17 p.m., Eli Reisman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35445/ > ----------------------------------------------------------- > > (Updated June 14, 2015, 10:17 p.m.) > > > Review request for samza. > > > Repository: samza > > > Description > ------- > > SAMZA-693: Very basic HDFS Producer service for Samza > > > Diffs > ----- > > build.gradle a5f54106a822dc91ff82270df27217a8765a0d80 > 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/test/org/apache/samza/system/hdfs/TestHdfsSystemProducer.scala > PRE-CREATION > samza-hdfs/src/test/resources/samza-hdfs-test-job.properties PRE-CREATION > settings.gradle bb07a3b84b14dcef94da1bb166eab6aa3d0026bb > > Diff: https://reviews.apache.org/r/35445/diff/ > > > Testing > ------- > > New unit test, but it's fairly rudimentary. Passes "./gradlew test" and > "./gradlew check" > > This only supplies an HDFS Producer, and this producer only writes > SequenceFiles of ByteWriteables so far. If the patch were accepted as-is, I'd > suggest future tickets for a matching HDFS Consumer, and a pluggable set of > output formats, configurable via HdfsConfig settings. > > On the upside, this patch has been tested on a real cluster with real data, > using several serdes, with good results. > > > Thanks, > > Eli Reisman > >
