----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53453/#review155495 -----------------------------------------------------------
Fix it, then Ship it! Is there any practical implementation of this interface . eg. adding to kafkasystemconsumer? Or is it expected in the future? samza-core/src/main/scala/org/apache/samza/checkpoint/OffsetManager.scala (line 243) <https://reviews.apache.org/r/53453/#comment225465> Ah.. I think I would prefer to add a comment explaining what you are grouping by instead of "returns map (systemName -> Map(ssp -> offset)". I couldn't figure out which is returning the 2-levelmap :) Also, groupBy(_._1.getSystem) is very hard to read. Maybe you can rewrite this code as : partitionOffsets.groupBy { case(ssp: SystemStreamPartition, offset: String) => ssp.getSystem() } and skip the comments fully?? samza-core/src/test/scala/org/apache/samza/checkpoint/TestOffsetManager.scala (line 292) <https://reviews.apache.org/r/53453/#comment225466> offsetManager.stop is missing - Navina Ramesh On Nov. 4, 2016, 11:23 p.m., Boris Shkolnik wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53453/ > ----------------------------------------------------------- > > (Updated Nov. 4, 2016, 11:23 p.m.) > > > Review request for samza. > > > Bugs: SAMZA-1042 > https://issues.apache.org/jira/browse/SAMZA-1042 > > > Repository: samza > > > Description > ------- > > Add optional interface for SystemConsumer checkpontListener() for checkpoint > notifications. > > > Diffs > ----- > > samza-api/src/main/java/org/apache/samza/checkpoint/CheckpointListener.java > PRE-CREATION > samza-api/src/main/java/org/apache/samza/system/SystemConsumer.java > 8dfcc7499659442aabd3085a8787475fe38f29db > samza-core/src/main/scala/org/apache/samza/checkpoint/OffsetManager.scala > c41eadb70f4675816245f7ac40f0db2fc16335f0 > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala > e0468ee89c89fd720834461771ebb36475475bcb > > samza-core/src/test/scala/org/apache/samza/checkpoint/TestOffsetManager.scala > cb78223f1b59a78bbeb1e42b5974670a53def504 > > Diff: https://reviews.apache.org/r/53453/diff/ > > > Testing > ------- > > gradlew test. > manual testing. > > > Thanks, > > Boris Shkolnik > >
