----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34500/#review85590 -----------------------------------------------------------
samza-sql-core/src/main/java/org/apache/samza/sql/api/data/EntityName.java <https://reviews.apache.org/r/34500/#comment137181> How is this "anonymous" system used? samza-sql-core/src/main/java/org/apache/samza/sql/api/operators/OperatorSource.java <https://reviews.apache.org/r/34500/#comment137270> OperatorSource and OperatorSink have the same method signatures. Is that even allowed in Java? It's kind of confusing, even though the implementation can be semantically different. samza-sql-core/src/main/java/org/apache/samza/sql/operators/OperatorTopology.java <https://reviews.apache.org/r/34500/#comment137271> Same question as before. Which method gets invoked when I do a <OperatorTopology_object>.getName() ? samza-sql-core/src/main/java/org/apache/samza/sql/operators/factory/SimpleOperatorImpl.java <https://reviews.apache.org/r/34500/#comment137187> Looks like you left behind some merge conflict statements in comments :) samza-sql-core/src/main/java/org/apache/samza/sql/operators/factory/TopologyBuilder.java <https://reviews.apache.org/r/34500/#comment137325> Can you add info on what an unbound input or unbound output is? I think it will be useful to add/move the comment from Line 205 and Line 218 here. samza-sql-core/src/main/java/org/apache/samza/sql/operators/factory/TopologyBuilder.java <https://reviews.apache.org/r/34500/#comment137339> Can we change this source() rather than stream() ? samza-sql-core/src/main/java/org/apache/samza/sql/operators/factory/TopologyBuilder.java <https://reviews.apache.org/r/34500/#comment137338> Can you explain when sink should be used? samza-sql-core/src/test/java/org/apache/samza/task/sql/UserCallbacksSqlTask.java <https://reviews.apache.org/r/34500/#comment137333> I thought TopologyBuilder was to abstract away the spec and provide a simplified API for a user implementing a simple SQL query. Imo, this still seems pretty involved for a user concerned with just defining a simple join query. I assumed we could have a builder pattern as below: ``` TopologyBuilder builder = TopologyBuilder .create() .join(window("stream1", 10), window("stream2", 10), List{joinKey1, joinKey2, ...}) .partition(partitionKey) .build() ``` The idea here is that the build statement order determines the topology. The builder just validates and chains them together. I can see that this can be a problem with running operators in parallel and possibly, make it hard for the user to understand the correct sequence of operators. I am wondering if you think this kind of a model is possible. It would greatly simplify the API for most users. Just wanted to put this comment out so that we can discuss further. samza-sql-core/src/test/java/org/apache/samza/task/sql/UserCallbacksSqlTask.java <https://reviews.apache.org/r/34500/#comment137337> Why do we need 2 instances of the TopologyBuilder here? I think this occurs because stream() and sink() method return OperatorSource type rather than a TopologyBuilder instance. How differently does the TopologyBuilder handle the OperatorSource and the OperatorSink ? - Navina Ramesh On May 20, 2015, 11:13 p.m., Yi Pan (Data Infrastructure) wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34500/ > ----------------------------------------------------------- > > (Updated May 20, 2015, 11:13 p.m.) > > > Review request for samza, Yan Fang, Chris Riccomini, Guozhang Wang, Milinda > Pathirage, Navina Ramesh, and Naveen Somasundaram. > > > Bugs: SAMZA-552 > https://issues.apache.org/jira/browse/SAMZA-552 > > > Repository: samza > > > Description > ------- > > SAMZA-552: added operator builder API > - The current operator builder only supports single output DAG topology yet > > > Diffs > ----- > > samza-sql-core/src/main/java/org/apache/samza/sql/api/data/EntityName.java > PRE-CREATION > samza-sql-core/src/main/java/org/apache/samza/sql/api/data/Table.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/api/operators/Operator.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/api/operators/OperatorCallback.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/api/operators/OperatorRouter.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/api/operators/OperatorSink.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/api/operators/OperatorSource.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/api/operators/SimpleOperator.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/data/IncomingMessageTuple.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/operators/OperatorTopology.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/operators/factory/NoopOperatorCallback.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/operators/factory/SimpleOperatorImpl.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/operators/factory/SimpleOperatorSpec.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/operators/factory/SimpleRouter.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/operators/factory/TopologyBuilder.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/operators/join/StreamStreamJoin.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/operators/join/StreamStreamJoinSpec.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/operators/partition/PartitionOp.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/operators/partition/PartitionSpec.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/operators/window/BoundedTimeWindow.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/operators/window/WindowSpec.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/task/sql/SimpleMessageCollector.java > PRE-CREATION > > samza-sql-core/src/test/java/org/apache/samza/task/sql/RandomWindowOperatorTask.java > PRE-CREATION > samza-sql-core/src/test/java/org/apache/samza/task/sql/StreamSqlTask.java > PRE-CREATION > > samza-sql-core/src/test/java/org/apache/samza/task/sql/UserCallbacksSqlTask.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/34500/diff/ > > > Testing > ------- > > ./gradlew clean build passed > > > Thanks, > > Yi Pan (Data Infrastructure) > >