----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36903/#review94193 -----------------------------------------------------------
samza-test/src/test/scala/org/apache/samza/test/integration/StreamTaskTest.scala (lines 47 - 49) <https://reviews.apache.org/r/36903/#comment148730> we need to remove the author information. :) And maybe add some java doc instead. My 2 cents: 1. If this is a real test, to be consistent, we may want to use TestStreamTask (begin with Test), or change all other TestSomething to SomethingTest (e.g. change TestStateful to StatefulTest) 2. If this is not a real test, I prefer something like StreamTaskUtil to be less ambiguous. samza-test/src/test/scala/org/apache/samza/test/integration/StreamTaskTest.scala (line 96) <https://reviews.apache.org/r/36903/#comment148740> is this tag used? samza-test/src/test/scala/org/apache/samza/test/integration/StreamTaskTest.scala (line 148) <https://reviews.apache.org/r/36903/#comment148741> same, is this used? samza-test/src/test/scala/org/apache/samza/test/integration/StreamTaskTest.scala (line 169) <https://reviews.apache.org/r/36903/#comment148742> There is no "TestJob". (I know, it is copy/paste issue :) samza-test/src/test/scala/org/apache/samza/test/integration/StreamTaskTest.scala (line 176) <https://reviews.apache.org/r/36903/#comment148752> why TestStateStoreTask here? I think you mean TestTask.awaitTaskReistered samza-test/src/test/scala/org/apache/samza/test/integration/TestShutdownContainer.scala (line 64) <https://reviews.apache.org/r/36903/#comment148745> From the description, it is not testing the Container Shutdown, actually it is testing the store restoring feature. samza-test/src/test/scala/org/apache/samza/test/integration/TestShutdownContainer.scala (line 66) <https://reviews.apache.org/r/36903/#comment148734> Since we already are doing the abstraction, is it possible to put the common config into StreamTastTest object? Becaue I see a lot of the same configs in ShutdownContainerTest and TestStatefulTask. samza-test/src/test/scala/org/apache/samza/test/integration/TestShutdownContainer.scala (lines 87 - 89) <https://reviews.apache.org/r/36903/#comment148755> in the 0.10.0, we do not have checkpoint factory, I believe samza-test/src/test/scala/org/apache/samza/test/integration/TestShutdownContainer.scala (lines 142 - 146) <https://reviews.apache.org/r/36903/#comment148754> are those two methods used anywhere? samza-test/src/test/scala/org/apache/samza/test/integration/TestShutdownContainer.scala (line 155) <https://reviews.apache.org/r/36903/#comment148758> how about adding override ? samza-test/src/test/scala/org/apache/samza/test/integration/TestShutdownContainer.scala (line 165) <https://reviews.apache.org/r/36903/#comment148759> how about adding override? samza-test/src/test/scala/org/apache/samza/test/integration/TestStatefulTask.scala (lines 88 - 91) <https://reviews.apache.org/r/36903/#comment148756> same samza-test/src/test/scala/org/apache/samza/test/integration/TestStatefulTask.scala (line 95) <https://reviews.apache.org/r/36903/#comment148757> actually i do not understand why we need a companion object here. We just use the default task number, 1. And awaitTaskRegistered and register methods are not used anywhere. samza-test/src/test/scala/org/apache/samza/test/integration/TestTask.scala (lines 32 - 34) <https://reviews.apache.org/r/36903/#comment148731> Instead of the author information, I think putting some java doc explaining this class/object will be better. samza-test/src/test/scala/org/apache/samza/test/integration/TestTask.scala (line 37) <https://reviews.apache.org/r/36903/#comment148749> rm ";" - Yan Fang On Aug. 4, 2015, 9:30 p.m., Yi Pan (Data Infrastructure) wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36903/ > ----------------------------------------------------------- > > (Updated Aug. 4, 2015, 9:30 p.m.) > > > Review request for samza, Yan Fang, Chinmay Soman, Chris Riccomini, and > Navina Ramesh. > > > Bugs: SAMZA-744 > https://issues.apache.org/jira/browse/SAMZA-744 > > > Repository: samza > > > Description > ------- > > SAMZA-744: shutdown stores before shutdown producers > > > Diffs > ----- > > build.gradle 0852adc4e8e0c2816afd1ebf433f1af6b44852f7 > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala > 27b2517048ad5730762506426ee7578c66181db8 > > samza-test/src/test/scala/org/apache/samza/test/integration/StreamTaskTest.scala > PRE-CREATION > > samza-test/src/test/scala/org/apache/samza/test/integration/TestShutdownContainer.scala > PRE-CREATION > > samza-test/src/test/scala/org/apache/samza/test/integration/TestStatefulTask.scala > ea702a919348305ff95ce0b4ca1996a13aff04ec > samza-test/src/test/scala/org/apache/samza/test/integration/TestTask.scala > PRE-CREATION > > Diff: https://reviews.apache.org/r/36903/diff/ > > > Testing > ------- > > ./bin/check-all.sh passed > > > Thanks, > > Yi Pan (Data Infrastructure) > >
