> On May 29, 2015, 1:28 a.m., Navina Ramesh wrote: > > samza-sql-core/src/main/java/org/apache/samza/sql/api/operators/OperatorSource.java, > > line 26 > > <https://reviews.apache.org/r/34500/diff/1/?file=965722#file965722line26> > > > > 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.
This should be fine as long as there aren't any classes implementing both interfaces (http://stackoverflow.com/questions/2801878/implementing-two-interfaces-in-a-class-with-same-method-which-interface-method). May be we can change method naming. > On May 29, 2015, 1:28 a.m., Navina Ramesh wrote: > > samza-sql-core/src/test/java/org/apache/samza/task/sql/UserCallbacksSqlTask.java, > > line 120 > > <https://reviews.apache.org/r/34500/diff/1/?file=965740#file965740line120> > > > > 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. I also agree with Navina here. I think we should make building topologies simple with the builder API. One complexity of current OperatorSpec based API is you need to create intermediate streams (EntityName)s to wire operators together. I think we should try to hide that complexity through the builder API. Even though source and sink hides that complexity to some extent, its better if we can completely remove that. - Milinda ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34500/#review85590 ----------------------------------------------------------- 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) > >
