----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47994/#review151356 -----------------------------------------------------------
samza-operator/src/main/java/org/apache/samza/operators/api/internal/Operators.java (line 29) <https://reviews.apache.org/r/47994/#comment219691> nit: Probably an IDE thing. We can be more specific about imports. samza-operator/src/main/java/org/apache/samza/operators/impl/ChainedOperators.java (line 43) <https://reviews.apache.org/r/47994/#comment219695> This is not a correctness issue. Often, Wouldn't it be nice to do traversal of the operator chain the same as that of the insertion order? samza-operator/src/main/java/org/apache/samza/operators/impl/ChainedOperators.java (line 64) <https://reviews.apache.org/r/47994/#comment219748> This function is neat! :-) samza-operator/src/main/java/org/apache/samza/operators/impl/ChainedOperators.java (line 74) <https://reviews.apache.org/r/47994/#comment219746> This part was not obvious to me on how objects were being re-used. Can you give me an example when this `if` will be true? samza-operator/src/main/java/org/apache/samza/operators/impl/OperatorFactory.java (line 55) <https://reviews.apache.org/r/47994/#comment219700> nit: Please add a javadoc, on what the returned `Entry` is, and what the boolean is? samza-operator/src/main/java/org/apache/samza/operators/impl/OperatorFactory.java (line 56) <https://reviews.apache.org/r/47994/#comment219721> Wouldn't it be ideal if the user of this factory did not have to care about whether an object was created the first time versus the object was already created by the factory? samza-operator/src/main/java/org/apache/samza/operators/impl/OperatorFactory.java (line 58) <https://reviews.apache.org/r/47994/#comment219745> Why do you need a check on `putIfAbsent` here given the `containsKey` check? It seems redundant. (IIUC, this class is not meant to be used in a multi-threaded mode. It's called from `init` which is single-threaded.) samza-operator/src/main/java/org/apache/samza/operators/impl/OperatorImpl.java (line 90) <https://reviews.apache.org/r/47994/#comment219750> Do we also need the `offset` of the incoming message that flows through each of these operators? Ideally, the offset should be a part of the context.(since, this RB is just for the wire-up, I'm certainly open to doing it later.) - Jagadish Venkatraman On Oct. 4, 2016, 8:05 a.m., Yi Pan (Data Infrastructure) wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47994/ > ----------------------------------------------------------- > > (Updated Oct. 4, 2016, 8:05 a.m.) > > > Review request for samza, Boris Shkolnik, Chris Pettitt, Chinmay Soman, Jake > Maes, Navina Ramesh, Jagadish Venkatraman, and Xinyu Liu. > > > Bugs: SAMZA-915 > https://issues.apache.org/jira/browse/SAMZA-915 > > > Repository: samza > > > Description > ------- > > SAMZA-915: implementation of StreamPipeline and operator runtime impl classes > > > Diffs > ----- > > > samza-operator/src/main/java/org/apache/samza/operators/api/MessageStream.java > PRE-CREATION > > samza-operator/src/main/java/org/apache/samza/operators/api/internal/Operators.java > PRE-CREATION > > samza-operator/src/main/java/org/apache/samza/operators/impl/ChainedOperators.java > PRE-CREATION > > samza-operator/src/main/java/org/apache/samza/operators/impl/OperatorFactory.java > PRE-CREATION > > samza-operator/src/main/java/org/apache/samza/operators/impl/OperatorImpl.java > PRE-CREATION > > samza-operator/src/main/java/org/apache/samza/operators/impl/join/PartialJoinOpImpl.java > PRE-CREATION > > samza-operator/src/main/java/org/apache/samza/operators/impl/window/SessionWindowImpl.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/operators/api/TestMessageStream.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/operators/api/internal/TestOperators.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/operators/impl/TestChainedOperators.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/operators/impl/TestOperatorFactory.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/operators/impl/TestOperatorImpl.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/operators/impl/TestOutputMessage.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/operators/impl/TestSimpleOperatorImpl.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/operators/impl/TestSinkOperatorImpl.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/operators/impl/data/serializers/SqlAvroSerdeTest.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/operators/impl/window/TestSessionWindowImpl.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/task/BroadcastOperatorTask.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/task/InputJsonSystemMessage.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/47994/diff/ > > > Testing > ------- > > ./gradlew clean build. > > > Thanks, > > Yi Pan (Data Infrastructure) > >
